netdev - Re: [PATCH 02/39] wimax: declarations for the in-kernel WiMAX API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 24 Nov 2008 16:12:22 -0800
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Inaky Perez-Gonzalez <inaky@...ux.intel.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 02/39] wimax: declarations for the in-kernel WiMAX API

On Mon, 24 Nov 2008 15:43:33 -0800
Inaky Perez-Gonzalez <inaky@...ux.intel.com> wrote:

> On Monday 24 November 2008, Stephen Hemminger wrote:
> > On Mon, 24 Nov 2008 13:50:25 -0800
> >
> > Inaky Perez-Gonzalez <inaky@...ux.intel.com> wrote:
> > > [...snip...]
> > > + */ 
> > > +struct wimax_dev {
> > > +	struct net_device *net_dev;
> > > +	struct list_head id_table_node;
> > > +	struct list_head pipe_list;
> > > +	struct wimax_pipe *pipe_msg;
> > > +	struct mutex mutex;		/* Protects all members and API calls */
> > > +	enum wimax_st state;
> > > +
> > > +	int (*op_msg_from_user)(struct wimax_dev *wimax_dev,
> > > +				const void *, size_t,
> > > +				const struct genl_info *info);
> > > +	int (*op_rfkill_sw_toggle)(struct wimax_dev *wimax_dev,
> > > +				   enum wimax_rf_state);
> > > +	int (*op_reset)(struct wimax_dev *wimax_dev);
> >
> > Move function pointers separate from data??
> 
> Most devices will have only a single adapter, so I thought it was not worth 
> the overhead of the double dereference when average you will have a single 
> copy.
> 
> > > +	struct genl_family gnl_family;
> >
> > Isn't family for all of wimax not per device?
> 
> Nope, it is per device. One generic netlink family per device (named "WiMAX 
> DEVICENAME"). 
> 
> Makes it very easy in user and kernel space, no need for the overhead of 
> having an extra attribute for the destination interface.
> 
> > > +	struct rfkill *rfkill;
> > > +	struct input_dev *rfkill_input;
> > > +	unsigned rf_hw:1;
> > > +	unsigned rf_sw:1;
> >
> > don't bother with bitfield overhead make them u8
> 
> sure
> 
> > > +	char name[32];
> > > +};
> >
> > Why have name in this data structure? Do you handle network device
> > renames properly?
> 
> This is a physical name, not the network interface name (recommended name in 
> the members documentation is DRIVERNAME:physical path.
> 
> The reason is it is used to register different things (rfkill device, input 
> device for rfkill, threads). It being tied to the physical path makes it easy 
> to map and not vulnerable to renames.
> 
> Now it might be too short, that's another matter.

Then shouldn't it be a 'struct device' and live in sysfs class hierarchy??
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html