Re: [RFC 06/16] ieee802154: 6lowpan: trivial checks at first

[Alexander Aring <alex.aring@gmail.com>]

On Wed, Aug 12, 2015 at 11:04:18AM +0200, Stefan Schmidt wrote:
> Hello.
> 
> On 03/08/15 08:23, Alexander Aring wrote:
> >This patch moves some trivial checks at first before calling
> >skb_share_check which could do some memcpy if the buffer is shared.
> >
> >Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> >---
> >  net/ieee802154/6lowpan/rx.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> >diff --git a/net/ieee802154/6lowpan/rx.c b/net/ieee802154/6lowpan/rx.c
> >index 7b6b038..11a5629 100644
> >--- a/net/ieee802154/6lowpan/rx.c
> >+++ b/net/ieee802154/6lowpan/rx.c
> >@@ -62,16 +62,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *wdev,
> >  	struct ieee802154_hdr hdr;
> >  	int ret;
> >+	if (skb->pkt_type == PACKET_OTHERHOST ||
> >+	    wdev->type != ARPHRD_IEEE802154)
> >+		goto drop;
> >+
> >  	skb = skb_share_check(skb, GFP_ATOMIC);
> >  	if (!skb)
> >  		goto drop;
> >-	if (skb->pkt_type == PACKET_OTHERHOST)
> >-		goto drop_skb;
> >-
> >-	if (wdev->type != ARPHRD_IEEE802154)
> >-		goto drop_skb;
> >-
> >  	if (ieee802154_hdr_peek_addrs(skb, &hdr) < 0)
> >  		goto drop_skb;
> 
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> 

Maybe I should switch also the conditions, means first check on
wdev-type. Because this function is called for all registered net_devices
(in my opinion) and do first check on type ARPHRD_IEEE802154 sounds more
resonable. We don't care about the other stuff (means different types).

In short: I think "wdev->type != ARPHRD_IEEE802154" is a more likely case
than "skb->pkt_type == PACKET_OTHERHOST".

- Alex