lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140826192521.GA4745@f1.synalogic.ca> Date: Tue, 26 Aug 2014 12:25:21 -0700 From: Benjamin Poirier <bpoirier@...e.de> To: Prashant Sreedharan <prashant@...adcom.com> Cc: Michael Chan <mchan@...adcom.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 3/3] tg3: Fix tx_pending checks for tg3_tso_bug On 2014/08/22 16:17, Prashant Sreedharan wrote: > Benjamin, thanks for the patch. Broadcom QA will be testing the changes. > Couple of comments below. > > segs = skb_gso_segment(skb, tp->dev->features & > > ~(NETIF_F_TSO | NETIF_F_TSO6)); > > - if (IS_ERR(segs) || !segs) > > + if (IS_ERR_OR_NULL(segs)) > > goto tg3_tso_bug_end; > > > > do { > > + unsigned int desc_cnt = skb_shinfo(segs)->nr_frags + 1; > > + > > nskb = segs; > > segs = segs->next; > > nskb->next = NULL; > > - tg3_start_xmit(nskb, tp->dev); > > + > > + if (tg3_tx_avail(tnapi) <= segs_remaining - 1 + desc_cnt && > > + skb_linearize(nskb)) { > > + nskb->next = segs; > > + segs = nskb; > > + do { > > + nskb = segs->next; > > + > > + dev_kfree_skb_any(segs); > > + segs = nskb; > > + } while (segs); > > If skb_linearize() fails need to increment the tp->tx_dropped count Sorry for the delay, while testing this error path I noticed a potential problem. There should be an additional check here to stop the queue with the default threshold. Otherwise, the netdev_err message at the start of __tg3_start_xmit() could be triggered when the next frame is transmitted. That is because the previous calls to __tg3_start_xmit() in tg3_tso_bug() may have been using a stop_thresh=segs_remaining that is < MAX_SKB_FRAGS + 1. > > > + goto tg3_tso_bug_end; > > + } > > + segs_remaining--; > > + if (segs_remaining) > > + __tg3_start_xmit(nskb, tp->dev, segs_remaining); > > To clarify passing segs_remaining will make sure the queue is never > stopped correct ? It makes sure the queue is not stopped before we are finished submitting all gso segments. This is what's alluded to in this part of the commit message: This puts us in the exceptional situation that a single skb that triggers tg3_tso_bug() may require the entire tx ring. [...] Likewise, usually the tx queue is stopped as soon as an skb with max frags may overrun it. Since the skbs submitted from tg3_tso_bug() use a controlled number of descriptors, the tx queue stop threshold may be lowered. > > > + else > > + tg3_start_xmit(nskb, tp->dev); > > } while (segs); > > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists