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

On Tue, Aug 11, 2009 at 10:39:58PM +0200, Johannes Berg wrote:

On Wed, 2009-08-12 at 00:23 +0400, Dmitry Eremin-Solenikov wrote:

quoted

quoted

That return value is strange.

The idea is that tx function can return info about errors during
transmit (busy channel, no ACK, etc).

Well, TX functions don't usually wait for anything so busy channel or no
ack wouldn't be possible to return there but more like a TX status, and
I think people are more used to NETDEV_TX_OK etc. return values. But
it's your choice to make, obviously. If you want to return specific
information like that though I would recommend making an enum so you get
at least some type checking (and things like 'return -1' are more
obviously wrong).

Currently we do all the work from special worker threads, so it's
possible for this callback to sleep. The error isn't yet propagated to
upper layers (there is a huge TODO there), anyway.

quoted

quoted

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.

What troubles w/ master netdev did you have had? I do still see the
master devices in mac80211. IIUC, it's planned to replace (from
management point of view) the mdev with wiphy instance, isn't it?

Mostly the userspace API was a mess, and you can't actually _do_
anything with the master netdev. It also doesn't see all the frames,
only outgoing. It's very strange.

We were using master netdevices for several purposes:
1) ip link add link mwpanX type wpan, so that we have out-of-box support
   for radio additions. That's really nice thing to have.

2) for SOCK_RAW implementation that can be used to send raw packets
   over-the-air/receive raw packets. I think we can use af_packet for
   this, but I'm still not sure about packet injection.

3) On the RX, we did send a clone of skb to the mdev and clones to slave
   interfaces, thus overcoming your last argument.

The biggest problem is that it really clutters up the userspace API
since you can't do any netdev things with it, it's just a placeholder.

In addition to that, you can't put anything into skb->cb, then push the
frame to the master netdev, and expect things in skb->cb to still be
there when the frame arrives at the master netdev. Not sure you do that
(I hope not because that would be very buggy), but eventually you'll
probably find that you do want that, etc.

Hmm. It works for us. Could you please tell me more about the problems
with skb->cb ?

IMHO it's just better practise to not use it in situations like this
where it can't be really used as a netdev.

We did use it as a netdev, however we can probably live without it.
I'll rethink this interface.

-- 
With best wishes
Dmitry