From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH V2 2/3] net: dsa: ksz: Factor out common tag code Date: Fri, 14 Dec 2018 21:11:35 +0100 Message-ID: <20181214201135.GB21591@lunn.ch> References: <20181213160159.18956-1-marex@denx.de> <20181213160159.18956-3-marex@denx.de> <20181214090525.GA5513@lunn.ch> <4ff4924c-415a-9555-762f-151d83035dab@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, f.fainelli@gmail.com, Tristram Ha , Vivien Didelot , Woojung Huh , "David S . Miller" To: Marek Vasut Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:48164 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730728AbeLNULj (ORCPT ); Fri, 14 Dec 2018 15:11:39 -0500 Content-Disposition: inline In-Reply-To: <4ff4924c-415a-9555-762f-151d83035dab@denx.de> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Dec 14, 2018 at 02:30:35PM +0100, Marek Vasut wrote: > On 12/14/2018 10:05 AM, Andrew Lunn wrote: > >> +static struct sk_buff *ksz_common_xmit(struct sk_buff *skb, > >> + struct net_device *dev, int len) > >> { > >> - struct dsa_port *dp = dsa_slave_to_port(dev); > >> struct sk_buff *nskb; > >> int padlen; > >> - u8 *tag; > >> > >> padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len; > >> > >> - if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) { > >> + if (skb_tailroom(skb) >= padlen + len) { > >> /* Let dsa_slave_xmit() free skb */ > >> if (__skb_put_padto(skb, skb->len + padlen, false)) > >> return NULL; > > > > Hi Marek > > Hi, > > > This can return NULL. > > > >> +static struct sk_buff *ksz9477_xmit(struct sk_buff *skb, > >> + struct net_device *dev) > >> +{ > >> + struct dsa_port *dp = dsa_slave_to_port(dev); > >> + struct sk_buff *nskb; > >> + u16 *tag; > >> + u8 *addr; > >> + > >> + nskb = ksz_common_xmit(skb, dev, KSZ9477_INGRESS_TAG_LEN); > > > > So here you need to check for NULL > > What do we do if the xmit returns NULL ? Return NULL to the caller. There are comments which explain this: static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev) { [snip] /* Transmit function may have to reallocate the original SKB, * in which case it must have freed it. Only free it here on error. */ nskb = p->xmit(skb, dev); if (!nskb) { kfree_skb(skb); return NETDEV_TX_OK; } Andrew