| Message ID | 1393851547-27876-5-git-send-email-phoebe.buckheister@itwm.fraunhofer.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi, This patch is fine. Increment and set of DSN value should be done by mac802154 layer. But there is also a known issue with increment of DSN value while 6lowpan fragmentation. On a fragmented packet 6lowpan calls dev_hard_header once, then many times lowpan_fragment_xmit, depending on the number of fragments which is needed. Each call of lowpan_fragment_xmit will have the same DSN value. Maybe we should put the increment of DSN in "mac802154/tx.c" before calling xmit callback from driver (Just an idea). What do you think about that? - Alex -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 03, 2014 at 03:06:01PM +0100, Alexander Aring wrote: > Hi, > > This patch is fine. Increment and set of DSN value should be done by > mac802154 layer. > > But there is also a known issue with increment of DSN value while > 6lowpan fragmentation. > > On a fragmented packet 6lowpan calls dev_hard_header once, then many > times lowpan_fragment_xmit, depending on the number of fragments which > is needed. > > Each call of lowpan_fragment_xmit will have the same DSN value. Maybe we > should put the increment of DSN in "mac802154/tx.c" before calling xmit > callback from driver (Just an idea). What do you think about that? > Or maybe we can move the dev_hard_header call in lowpan_fragment_xmit and lowpan_xmit if this is possible. But then we doesn't know the mac header length in lowpan_skb_fragmentation. Strange... - Alex -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On a fragmented packet 6lowpan calls dev_hard_header once, then many > times lowpan_fragment_xmit, depending on the number of fragments which > is needed. To me, the obvious answer would be "just don't do that". Each fragment is a distinct packet, so it should not reuse a header that was created for another packet in the first place. > Each call of lowpan_fragment_xmit will have the same DSN value. Maybe > we should put the increment of DSN in "mac802154/tx.c" before calling > xmit callback from driver (Just an idea). What do you think about > that? That would work. Crypto will have to set it's own frame counter there anyway, since reordering in the wpan queue would otherwise cause unexpected packet drops. I don't see that need for the DSN though. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 3 Mar 2014 15:10:27 +0100 Alexander Aring <alex.aring@gmail.com> wrote: > Or maybe we can move the dev_hard_header call in lowpan_fragment_xmit > and lowpan_xmit if this is possible. But then we doesn't know the mac > header length in lowpan_skb_fragmentation. Strange... You could have lowpan_fragment_xmit create an entirely new skb, call dev_hard_header, determine how many bytes you can pack into that skb and then do that. If you can transmit that fragment, return the number of bytes transmitted, if you can't, return -ENOBUFS or whatever seems more appropriate. Ideally, also ask mac802154 (somehow, not sure how) about how many bytes you can fit into the fragment instead of using a fixed maximum fragment size. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 03, 2014 at 03:12:19PM +0100, Phoebe Buckheister wrote: > > On a fragmented packet 6lowpan calls dev_hard_header once, then many > > times lowpan_fragment_xmit, depending on the number of fragments which > > is needed. > > To me, the obvious answer would be "just don't do that". Each fragment > is a distinct packet, so it should not reuse a header that was created > for another packet in the first place. > mhh, then the best solution would be to drop the copy of the mac header and generate it there. - Alex -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h index bc14665..503cb9f 100644 --- a/include/net/ieee802154_netdev.h +++ b/include/net/ieee802154_netdev.h @@ -85,7 +85,6 @@ struct ieee802154_frag_info { struct ieee802154_mac_cb { u8 lqi; u8 flags; - u8 seq; struct ieee802154_frag_info frag_info; }; diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c index b413e4e..7ebc300 100644 --- a/net/ieee802154/6lowpan_rtnl.c +++ b/net/ieee802154/6lowpan_rtnl.c @@ -117,7 +117,6 @@ static int lowpan_header_create(struct sk_buff *skb, * this isn't implemented in mainline yet, so currently we assign 0xff */ mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA; - mac_cb(skb)->seq = ieee802154_mlme_ops(dev)->get_dsn(dev); /* prepare wpan address data */ sa.addr_type = IEEE802154_ADDR_LONG; diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c index 5fcb817..6480510 100644 --- a/net/ieee802154/dgram.c +++ b/net/ieee802154/dgram.c @@ -253,7 +253,6 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk, if (ro->want_ack) mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ; - mac_cb(skb)->seq = ieee802154_mlme_ops(dev)->get_dsn(dev); err = dev_hard_header(skb, dev, ETH_P_IEEE802154, &ro->dst_addr, ro->bound ? &ro->src_addr : NULL, size); if (err < 0) diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c index 1ebe0e3..b7e47ac 100644 --- a/net/mac802154/wpan.c +++ b/net/mac802154/wpan.c @@ -111,7 +111,7 @@ static int mac802154_header_create(struct sk_buff *skb, return -EINVAL; hdr.fc = mac_cb_type(skb); - hdr.seq = mac_cb(skb)->seq; + hdr.seq = ieee802154_mlme_ops(dev)->get_dsn(dev); if (mac_cb_is_ackreq(skb)) hdr.fc |= IEEE802154_FC_ACK_REQ; if (mac_cb_is_secen(skb))
The seq member is only used to initialize the sequence number field of the 802.15.4 header. This field has relevance only for low-level functionality like frame acknowledgement and is of no importance to upper layers. Upper layers should not be allowed to set this field at all. Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de> --- include/net/ieee802154_netdev.h | 1 - net/ieee802154/6lowpan_rtnl.c | 1 - net/ieee802154/dgram.c | 1 - net/mac802154/wpan.c | 2 +- 4 files changed, 1 insertion(+), 4 deletions(-)