netdev - Re: [PATCH 1/2] mac802154: add a software MAC 802.15.4 implementation

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]

Message-Id: <1249914466.32614.13.camel@johannes.local>
Date:	Mon, 10 Aug 2009 16:27:46 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Dmitry Eremin-Solenikov <dbaryshkov@...il.com>
Cc:	netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
	Sergey Lapin <slapin@...fans.org>
Subject: Re: [PATCH 1/2] mac802154: add a software MAC 802.15.4
 implementation

On Mon, 2009-08-10 at 18:16 +0400, Dmitry Eremin-Solenikov wrote:
> Some of available devices are just dump radios implementing IEEE 802.15.4

dump -> dumb

> Note this implementaion is neither certified, nor feature complete!

implementation

> + * @tx: Handler that 802.15.4 module calls for each transmitted frame.
> + *      skb cntains the buffer starting from the IEEE 802.15.4 header.
> + *      The low-level driver should send the frame based on available
> + *      configuration.
> + *      This function should return zero or negative errno.

That return value is strange.

> +++ b/net/Makefile
> @@ -61,6 +61,7 @@ ifneq ($(CONFIG_DCB),)
>  obj-y				+= dcb/
>  endif
>  obj-y				+= ieee802154/
> +obj-y				+= mac802154/

I think you should use obj-$(CONFIG_MAC802154) as there's no need to
recurse into the dir unless that's selected.

But does it make sense to actually have this little code as a separate
dir/module?

> +	  Note: this implementation is neither certified, nor feature
> +	  complete! We do not garantee that it is compatible w/ other
> +	  implementations, etc.

guarantee

But generally, you don't have to write that anyway due to the license.

> +	  If you plan to use HardMAC IEEE 802.15.4 devices, you can
> +          say N here. Alternatievly you can say M to compile it as
> +	  module.

white space gone wrong

> +	struct net_device *netdev; /* mwpanX device */
> +	int open_count;
> +	/* As in mac80211 slaves list is modified:
> +	 * 1) under the RTNL
> +	 * 2) protected by slaves_mtx;
> +	 * 3) in an RCU manner
> +	 *
> +	 * So atomic readers can use any of this protection methods
> +	 */
> +	struct list_head	slaves;

heh.

> +static int ieee802154_master_open(struct net_device *dev)
> +{

We've had no end to trouble with the master netdev, I suggest you look
into the current mac80211 code and see if you can get rid of it.

> +struct ieee802154_dev *ieee802154_alloc_device(size_t priv_size,
> +		struct ieee802154_ops *ops)
> +{

> +	if (!try_module_get(ops->owner)) {

That isn't necessary since the module is just calling your function. In
fact, doing this removes the ability to rmmod any module using this
which is bogus.

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)