| Message ID | 1370406254-6341-2-git-send-email-stephen@networkplumber.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 05 Jun 2013 at 04:24 GMT, Stephen Hemminger <stephen@networkplumber.org> wrote: > skb1 = skb_clone(skb, GFP_ATOMIC); > - rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); > - if (rc == NETDEV_TX_OK) > - rc = rc1; > + if (skb1) { > + rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); > + if (rc == NETDEV_TX_OK) > + rc = rc1; > + } If OOM, shouldn't we exit immediately instead of continue handle the next one? -- 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
Acked-by: David L Stevens <dlstevens@us.ibm.com> > From: Stephen Hemminger <stephen@networkplumber.org> > skb_clone can fail if out of memory. Just skip the fanout. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > > --- > Does not impact stable, this was introduced by the multiple > destination code. > --- > drivers/net/vxlan.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index 536082a..9085c81 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -1197,9 +1197,11 @@ static netdev_tx_t vxlan_xmit(struct sk_buff > *skb, struct net_device *dev) > struct sk_buff *skb1; > > skb1 = skb_clone(skb, GFP_ATOMIC); > - rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); > - if (rc == NETDEV_TX_OK) > - rc = rc1; > + if (skb1) { > + rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); > + if (rc == NETDEV_TX_OK) > + rc = rc1; > + } > } > > rc1 = vxlan_xmit_one(skb, dev, rdst0, did_rsc); > -- > 1.7.10.4 > > -- > 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 > -- 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
netdev-owner@vger.kernel.org wrote on 06/05/2013 02:59:56 AM: > From: Cong Wang <xiyou.wangcong@gmail.com> > To: netdev@vger.kernel.org > Date: 06/05/2013 03:00 AM > Subject: Re: [PATCH net-next 02/10] vxlan: handle skb_clone failure > Sent by: netdev-owner@vger.kernel.org > > On Wed, 05 Jun 2013 at 04:24 GMT, Stephen Hemminger > <stephen@networkplumber.org> wrote: > > skb1 = skb_clone(skb, GFP_ATOMIC); > > - rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); > > - if (rc == NETDEV_TX_OK) > > - rc = rc1; > > + if (skb1) { > > + rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); > > + if (rc == NETDEV_TX_OK) > > + rc = rc1; > > + } > > If OOM, shouldn't we exit immediately instead of continue handle the > next one? It could be a temporary condition; trying again doesn't really hurt, in case some do go through. So, I prefer this, but could live with bailing for all destinations, too. +-DLS -- 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 Wed, Jun 5, 2013 at 10:05 PM, David Stevens <dlstevens@us.ibm.com> wrote: > netdev-owner@vger.kernel.org wrote on 06/05/2013 02:59:56 AM: > >> From: Cong Wang <xiyou.wangcong@gmail.com> >> If OOM, shouldn't we exit immediately instead of continue handle the >> next one? > > It could be a temporary condition; trying again doesn't really hurt, > in case some do go through. So, I prefer this, but could live with > bailing for all destinations, too. > The problem we don't know if it is temporary or not, unless mm provides EAGAIN? -- 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 Thu, 6 Jun 2013 08:47:49 +0800 Cong Wang <xiyou.wangcong@gmail.com> wrote: > On Wed, Jun 5, 2013 at 10:05 PM, David Stevens <dlstevens@us.ibm.com> wrote: > > netdev-owner@vger.kernel.org wrote on 06/05/2013 02:59:56 AM: > > > >> From: Cong Wang <xiyou.wangcong@gmail.com> > >> If OOM, shouldn't we exit immediately instead of continue handle the > >> next one? > > > > It could be a temporary condition; trying again doesn't really hurt, > > in case some do go through. So, I prefer this, but could live with > > bailing for all destinations, too. > > > > The problem we don't know if it is temporary or not, unless mm provides > EAGAIN? > -- > 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 There is no EAGAIN in the network transmit path, it is not called from user context. -- 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/drivers/net/vxlan.c b/drivers/net/vxlan.c index 536082a..9085c81 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -1197,9 +1197,11 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) struct sk_buff *skb1; skb1 = skb_clone(skb, GFP_ATOMIC); - rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); - if (rc == NETDEV_TX_OK) - rc = rc1; + if (skb1) { + rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc); + if (rc == NETDEV_TX_OK) + rc = rc1; + } } rc1 = vxlan_xmit_one(skb, dev, rdst0, did_rsc);
skb_clone can fail if out of memory. Just skip the fanout. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- Does not impact stable, this was introduced by the multiple destination code. --- drivers/net/vxlan.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)