* [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets @ 2012-04-12 9:07 Will Deacon 2012-04-12 9:20 ` Eric Dumazet 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2012-04-12 9:07 UTC (permalink / raw) To: netdev; +Cc: Will Deacon, Steve Glendinning The SMSC911x ethernet controller provides a mechanism for quickly skipping to the start of the next frame in the receive FIFO, however the current code passes the number of words to a function that expects the number of bytes. This can corrupt the FIFO head in the case that the fastforward mechanism is not used. This patch fixes the callers of smsc911x_rx_fastforward to pass the correct data size. Cc: Steve Glendinning <steve.glendinning@smsc.com> Signed-off-by: Will Deacon <will.deacon@arm.com> --- drivers/net/ethernet/smsc/smsc911x.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 4a69710..b5599bc 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -1228,7 +1228,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) "Discarding packet with error bit set"); /* Packet has an error, discard it and continue with * the next */ - smsc911x_rx_fastforward(pdata, pktwords); + smsc911x_rx_fastforward(pdata, pktlength); dev->stats.rx_dropped++; continue; } @@ -1238,7 +1238,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) SMSC_WARN(pdata, rx_err, "Unable to allocate skb for rx packet"); /* Drop the packet and stop this polling iteration */ - smsc911x_rx_fastforward(pdata, pktwords); + smsc911x_rx_fastforward(pdata, pktlength); dev->stats.rx_dropped++; break; } -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 9:07 [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets Will Deacon @ 2012-04-12 9:20 ` Eric Dumazet 2012-04-12 12:53 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Eric Dumazet @ 2012-04-12 9:20 UTC (permalink / raw) To: Will Deacon; +Cc: netdev, Steve Glendinning On Thu, 2012-04-12 at 10:07 +0100, Will Deacon wrote: > The SMSC911x ethernet controller provides a mechanism for quickly > skipping to the start of the next frame in the receive FIFO, however > the current code passes the number of words to a function that expects > the number of bytes. This can corrupt the FIFO head in the case that > the fastforward mechanism is not used. > > This patch fixes the callers of smsc911x_rx_fastforward to pass the > correct data size. > > Cc: Steve Glendinning <steve.glendinning@smsc.com> > Signed-off-by: Will Deacon <will.deacon@arm.com> > --- > drivers/net/ethernet/smsc/smsc911x.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > index 4a69710..b5599bc 100644 > --- a/drivers/net/ethernet/smsc/smsc911x.c > +++ b/drivers/net/ethernet/smsc/smsc911x.c > @@ -1228,7 +1228,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > "Discarding packet with error bit set"); > /* Packet has an error, discard it and continue with > * the next */ > - smsc911x_rx_fastforward(pdata, pktwords); > + smsc911x_rx_fastforward(pdata, pktlength); > dev->stats.rx_dropped++; > continue; > } > @@ -1238,7 +1238,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > SMSC_WARN(pdata, rx_err, > "Unable to allocate skb for rx packet"); > /* Drop the packet and stop this polling iteration */ > - smsc911x_rx_fastforward(pdata, pktwords); > + smsc911x_rx_fastforward(pdata, pktlength); > dev->stats.rx_dropped++; > break; > } Hum, looking at this driver, I see wrong code in lines 1246/1247 skb->data = skb->head; skb_reset_tail_pointer(skb); I suspect its hiding a buffer overflow bug or something. netdev_alloc_skb() reserved NET_SKB_PAD bytes. A driver should not un-reserve this headroom, or some networking setups can be very slow. So pdata->ops->rx_readfifo(pdata, (unsigned int *)skb->head, pktwords); also should be fixed to use skb->data instead. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 9:20 ` Eric Dumazet @ 2012-04-12 12:53 ` Will Deacon 2012-04-12 13:06 ` Eric Dumazet 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2012-04-12 12:53 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, Steve Glendinning Hi Eric, Thanks for taking a look. On Thu, Apr 12, 2012 at 10:20:48AM +0100, Eric Dumazet wrote: > On Thu, 2012-04-12 at 10:07 +0100, Will Deacon wrote: > > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > > index 4a69710..b5599bc 100644 > > --- a/drivers/net/ethernet/smsc/smsc911x.c > > +++ b/drivers/net/ethernet/smsc/smsc911x.c > > @@ -1228,7 +1228,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > > "Discarding packet with error bit set"); > > /* Packet has an error, discard it and continue with > > * the next */ > > - smsc911x_rx_fastforward(pdata, pktwords); > > + smsc911x_rx_fastforward(pdata, pktlength); > > dev->stats.rx_dropped++; > > continue; > > } [...] > Hum, looking at this driver, I see wrong code in lines 1246/1247 > > skb->data = skb->head; > skb_reset_tail_pointer(skb); > > I suspect its hiding a buffer overflow bug or something. Yes, you're right. > netdev_alloc_skb() reserved NET_SKB_PAD bytes. A driver should not > un-reserve this headroom, or some networking setups can be very slow. > > So > > pdata->ops->rx_readfifo(pdata, > (unsigned int *)skb->head, pktwords); > > also should be fixed to use skb->data instead. Right, this seems to do the trick (and can replace my original patch by actually passing in the number of words to the fastforward function). I'm not sure whether the skb_trim is really required, but it makes the data format slightly clearer. It would be nice to get some input from Steve, but his email address seems to be bouncing at the moment. Will diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 4a69710..3f43c24 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -1166,10 +1166,8 @@ smsc911x_rx_counterrors(struct net_device *dev, unsigned int rxstat) /* Quickly dumps bad packets */ static void -smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktbytes) +smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktwords) { - unsigned int pktwords = (pktbytes + NET_IP_ALIGN + 3) >> 2; - if (likely(pktwords >= 4)) { unsigned int timeout = 500; unsigned int val; @@ -1233,7 +1231,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) continue; } - skb = netdev_alloc_skb(dev, pktlength + NET_IP_ALIGN); + skb = netdev_alloc_skb(dev, pktwords << 2); if (unlikely(!skb)) { SMSC_WARN(pdata, rx_err, "Unable to allocate skb for rx packet"); @@ -1243,21 +1241,19 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) break; } - skb->data = skb->head; - skb_reset_tail_pointer(skb); - - /* Align IP on 16B boundary */ - skb_reserve(skb, NET_IP_ALIGN); - skb_put(skb, pktlength - 4); + skb_put(skb, pktwords << 2); pdata->ops->rx_readfifo(pdata, - (unsigned int *)skb->head, pktwords); + (unsigned int *)skb->data, pktwords); + skb_pull(skb, NET_IP_ALIGN); + skb_trim(skb, pktlength); + skb->protocol = eth_type_trans(skb, dev); skb_checksum_none_assert(skb); netif_receive_skb(skb); /* Update counters */ dev->stats.rx_packets++; - dev->stats.rx_bytes += (pktlength - 4); + dev->stats.rx_bytes += pktlength; } /* Return total received packets */ @@ -1565,7 +1561,7 @@ static int smsc911x_open(struct net_device *dev) smsc911x_reg_write(pdata, FIFO_INT, temp); /* set RX Data offset to 2 bytes for alignment */ - smsc911x_reg_write(pdata, RX_CFG, (2 << 8)); + smsc911x_reg_write(pdata, RX_CFG, (NET_IP_ALIGN << 8)); /* enable NAPI polling before enabling RX interrupts */ napi_enable(&pdata->napi); ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 12:53 ` Will Deacon @ 2012-04-12 13:06 ` Eric Dumazet 2012-04-12 13:47 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Eric Dumazet @ 2012-04-12 13:06 UTC (permalink / raw) To: Will Deacon; +Cc: netdev, Steve Glendinning On Thu, 2012-04-12 at 13:53 +0100, Will Deacon wrote: > Hi Eric, > > Thanks for taking a look. > > On Thu, Apr 12, 2012 at 10:20:48AM +0100, Eric Dumazet wrote: > > On Thu, 2012-04-12 at 10:07 +0100, Will Deacon wrote: > > > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > > > index 4a69710..b5599bc 100644 > > > --- a/drivers/net/ethernet/smsc/smsc911x.c > > > +++ b/drivers/net/ethernet/smsc/smsc911x.c > > > @@ -1228,7 +1228,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > > > "Discarding packet with error bit set"); > > > /* Packet has an error, discard it and continue with > > > * the next */ > > > - smsc911x_rx_fastforward(pdata, pktwords); > > > + smsc911x_rx_fastforward(pdata, pktlength); > > > dev->stats.rx_dropped++; > > > continue; > > > } > > [...] > > > Hum, looking at this driver, I see wrong code in lines 1246/1247 > > > > skb->data = skb->head; > > skb_reset_tail_pointer(skb); > > > > I suspect its hiding a buffer overflow bug or something. > > Yes, you're right. > > > netdev_alloc_skb() reserved NET_SKB_PAD bytes. A driver should not > > un-reserve this headroom, or some networking setups can be very slow. > > > > So > > > > pdata->ops->rx_readfifo(pdata, > > (unsigned int *)skb->head, pktwords); > > > > also should be fixed to use skb->data instead. > > Right, this seems to do the trick (and can replace my original patch by > actually passing in the number of words to the fastforward function). I'm > not sure whether the skb_trim is really required, but it makes the data > format slightly clearer. > > It would be nice to get some input from Steve, but his email address seems > to be bouncing at the moment. > > Will > > > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > index 4a69710..3f43c24 100644 > --- a/drivers/net/ethernet/smsc/smsc911x.c > +++ b/drivers/net/ethernet/smsc/smsc911x.c > @@ -1166,10 +1166,8 @@ smsc911x_rx_counterrors(struct net_device *dev, unsigned int rxstat) > > /* Quickly dumps bad packets */ > static void > -smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktbytes) > +smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktwords) > { > - unsigned int pktwords = (pktbytes + NET_IP_ALIGN + 3) >> 2; > - > if (likely(pktwords >= 4)) { > unsigned int timeout = 500; > unsigned int val; > @@ -1233,7 +1231,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > continue; > } > > - skb = netdev_alloc_skb(dev, pktlength + NET_IP_ALIGN); > + skb = netdev_alloc_skb(dev, pktwords << 2); > if (unlikely(!skb)) { > SMSC_WARN(pdata, rx_err, > "Unable to allocate skb for rx packet"); > @@ -1243,21 +1241,19 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > break; > } > > - skb->data = skb->head; > - skb_reset_tail_pointer(skb); > - > - /* Align IP on 16B boundary */ > - skb_reserve(skb, NET_IP_ALIGN); > - skb_put(skb, pktlength - 4); > + skb_put(skb, pktwords << 2); You could remove this line and do it after skb_reserve() ? > pdata->ops->rx_readfifo(pdata, > - (unsigned int *)skb->head, pktwords); > + (unsigned int *)skb->data, pktwords); > + skb_pull(skb, NET_IP_ALIGN); skb_reserve(skb, NET_IP_ALIGN); skb_put(skb, pktlength - 4); > + skb_trim(skb, pktlength); and remove this skb_trim() > + > skb->protocol = eth_type_trans(skb, dev); > skb_checksum_none_assert(skb); > netif_receive_skb(skb); > > /* Update counters */ > dev->stats.rx_packets++; > - dev->stats.rx_bytes += (pktlength - 4); > + dev->stats.rx_bytes += pktlength; Some drivers account for the FCS, some dont, I guess you can leave the line as is > } > > /* Return total received packets */ > @@ -1565,7 +1561,7 @@ static int smsc911x_open(struct net_device *dev) > smsc911x_reg_write(pdata, FIFO_INT, temp); > > /* set RX Data offset to 2 bytes for alignment */ > - smsc911x_reg_write(pdata, RX_CFG, (2 << 8)); > + smsc911x_reg_write(pdata, RX_CFG, (NET_IP_ALIGN << 8)); Good ;) > > /* enable NAPI polling before enabling RX interrupts */ > napi_enable(&pdata->napi); > Thanks ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 13:06 ` Eric Dumazet @ 2012-04-12 13:47 ` Will Deacon 2012-04-12 14:01 ` Eric Dumazet 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2012-04-12 13:47 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, Steve Glendinning On Thu, Apr 12, 2012 at 02:06:03PM +0100, Eric Dumazet wrote: > On Thu, 2012-04-12 at 13:53 +0100, Will Deacon wrote: > > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > > index 4a69710..3f43c24 100644 > > --- a/drivers/net/ethernet/smsc/smsc911x.c > > +++ b/drivers/net/ethernet/smsc/smsc911x.c > > @@ -1166,10 +1166,8 @@ smsc911x_rx_counterrors(struct net_device *dev, unsigned int rxstat) [...] > > - skb->data = skb->head; > > - skb_reset_tail_pointer(skb); > > - > > - /* Align IP on 16B boundary */ > > - skb_reserve(skb, NET_IP_ALIGN); > > - skb_put(skb, pktlength - 4); > > + skb_put(skb, pktwords << 2); > > You could remove this line and do it after skb_reserve() ? > > > pdata->ops->rx_readfifo(pdata, > > - (unsigned int *)skb->head, pktwords); > > + (unsigned int *)skb->data, pktwords); > > + skb_pull(skb, NET_IP_ALIGN); > > skb_reserve(skb, NET_IP_ALIGN); I don't think we want an skb_reserve at all, since the hardware shifts the data in the RX FIFO, meaning that we will read two bytes of 0 anyway before valid data. > skb_put(skb, pktlength - 4); I can move the put here if you like, but we need to use pktwords << 2 to make sure that we read the leading and trailing zeroes inserted by the hardware. > > + skb_trim(skb, pktlength); > > and remove this skb_trim() Can do, just thought it might help illustrate that we might have some padding at the end. > > /* Update counters */ > > dev->stats.rx_packets++; > > - dev->stats.rx_bytes += (pktlength - 4); > > + dev->stats.rx_bytes += pktlength; > > Some drivers account for the FCS, some dont, I guess you can leave the > line as is Sure. Thanks for the comments, Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 13:47 ` Will Deacon @ 2012-04-12 14:01 ` Eric Dumazet 2012-04-12 15:54 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Eric Dumazet @ 2012-04-12 14:01 UTC (permalink / raw) To: Will Deacon; +Cc: netdev, Steve Glendinning On Thu, 2012-04-12 at 14:47 +0100, Will Deacon wrote: > > I don't think we want an skb_reserve at all, since the hardware shifts the > data in the RX FIFO, meaning that we will read two bytes of 0 anyway before > valid data. > > > skb_put(skb, pktlength - 4); > > I can move the put here if you like, but we need to use pktwords << 2 to > make sure that we read the leading and trailing zeroes inserted by the > hardware. before calling linux stack, you'll have to skip those 2 bytes. This is skb_reserve() purpose. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 14:01 ` Eric Dumazet @ 2012-04-12 15:54 ` Will Deacon 2012-04-12 16:08 ` Eric Dumazet 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2012-04-12 15:54 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, Steve Glendinning On Thu, Apr 12, 2012 at 03:01:39PM +0100, Eric Dumazet wrote: > On Thu, 2012-04-12 at 14:47 +0100, Will Deacon wrote: > > > > > I don't think we want an skb_reserve at all, since the hardware shifts the > > data in the RX FIFO, meaning that we will read two bytes of 0 anyway before > > valid data. > > > > > skb_put(skb, pktlength - 4); > > > > I can move the put here if you like, but we need to use pktwords << 2 to > > make sure that we read the leading and trailing zeroes inserted by the > > hardware. > > before calling linux stack, you'll have to skip those 2 bytes. > > This is skb_reserve() purpose. Gotcha, so I can lose the pull too. Here's an updated patch with log, thanks for the help. Will Author: Will Deacon <will.deacon@arm.com> Date: Thu Apr 12 13:54:17 2012 +0100 net: smsc911x: fix skb handling in receive path The SMSC911x driver resets the ->head, ->data and ->tail pointers in the skb on the reset path in order to avoid buffer overflow due to packet padding performed by the hardware. This patch fixes the receive path so that the skb pointers are fixed up after the data has been read from the device, The error path is also fixed to use number of words consistently and prevent erroneous FIFO fastforwarding when skipping over bad data. Signed-off-by: Will Deacon <will.deacon@arm.com> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 4a69710..5aa2dbe 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -1166,10 +1166,8 @@ smsc911x_rx_counterrors(struct net_device *dev, unsigned int rxstat) /* Quickly dumps bad packets */ static void -smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktbytes) +smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktwords) { - unsigned int pktwords = (pktbytes + NET_IP_ALIGN + 3) >> 2; - if (likely(pktwords >= 4)) { unsigned int timeout = 500; unsigned int val; @@ -1233,7 +1231,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) continue; } - skb = netdev_alloc_skb(dev, pktlength + NET_IP_ALIGN); + skb = netdev_alloc_skb(dev, pktwords << 2); if (unlikely(!skb)) { SMSC_WARN(pdata, rx_err, "Unable to allocate skb for rx packet"); @@ -1243,14 +1241,12 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) break; } - skb->data = skb->head; - skb_reset_tail_pointer(skb); + pdata->ops->rx_readfifo(pdata, + (unsigned int *)skb->data, pktwords); /* Align IP on 16B boundary */ skb_reserve(skb, NET_IP_ALIGN); skb_put(skb, pktlength - 4); - pdata->ops->rx_readfifo(pdata, - (unsigned int *)skb->head, pktwords); skb->protocol = eth_type_trans(skb, dev); skb_checksum_none_assert(skb); netif_receive_skb(skb); @@ -1565,7 +1561,7 @@ static int smsc911x_open(struct net_device *dev) smsc911x_reg_write(pdata, FIFO_INT, temp); /* set RX Data offset to 2 bytes for alignment */ - smsc911x_reg_write(pdata, RX_CFG, (2 << 8)); + smsc911x_reg_write(pdata, RX_CFG, (NET_IP_ALIGN << 8)); /* enable NAPI polling before enabling RX interrupts */ napi_enable(&pdata->napi); ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 15:54 ` Will Deacon @ 2012-04-12 16:08 ` Eric Dumazet 2012-04-12 17:03 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Eric Dumazet @ 2012-04-12 16:08 UTC (permalink / raw) To: Will Deacon; +Cc: netdev, Steve Glendinning On Thu, 2012-04-12 at 16:54 +0100, Will Deacon wrote: > Gotcha, so I can lose the pull too. Here's an updated patch with log, thanks > for the help. > > Will > > > Author: Will Deacon <will.deacon@arm.com> > Date: Thu Apr 12 13:54:17 2012 +0100 > > net: smsc911x: fix skb handling in receive path > > The SMSC911x driver resets the ->head, ->data and ->tail pointers in the > skb on the reset path in order to avoid buffer overflow due to packet > padding performed by the hardware. > > This patch fixes the receive path so that the skb pointers are fixed up > after the data has been read from the device, The error path is also > fixed to use number of words consistently and prevent erroneous FIFO > fastforwarding when skipping over bad data. > > Signed-off-by: Will Deacon <will.deacon@arm.com> > > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > index 4a69710..5aa2dbe 100644 > --- a/drivers/net/ethernet/smsc/smsc911x.c > +++ b/drivers/net/ethernet/smsc/smsc911x.c > @@ -1166,10 +1166,8 @@ smsc911x_rx_counterrors(struct net_device *dev, unsigned int rxstat) > > /* Quickly dumps bad packets */ > static void > -smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktbytes) > +smsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int pktwords) > { > - unsigned int pktwords = (pktbytes + NET_IP_ALIGN + 3) >> 2; > - > if (likely(pktwords >= 4)) { > unsigned int timeout = 500; > unsigned int val; > @@ -1233,7 +1231,7 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > continue; > } > > - skb = netdev_alloc_skb(dev, pktlength + NET_IP_ALIGN); > + skb = netdev_alloc_skb(dev, pktwords << 2); > if (unlikely(!skb)) { > SMSC_WARN(pdata, rx_err, > "Unable to allocate skb for rx packet"); > @@ -1243,14 +1241,12 @@ static int smsc911x_poll(struct napi_struct *napi, int budget) > break; > } > > - skb->data = skb->head; > - skb_reset_tail_pointer(skb); > + pdata->ops->rx_readfifo(pdata, > + (unsigned int *)skb->data, pktwords); > > /* Align IP on 16B boundary */ > skb_reserve(skb, NET_IP_ALIGN); > skb_put(skb, pktlength - 4); > - pdata->ops->rx_readfifo(pdata, > - (unsigned int *)skb->head, pktwords); > skb->protocol = eth_type_trans(skb, dev); > skb_checksum_none_assert(skb); > netif_receive_skb(skb); > @@ -1565,7 +1561,7 @@ static int smsc911x_open(struct net_device *dev) > smsc911x_reg_write(pdata, FIFO_INT, temp); > > /* set RX Data offset to 2 bytes for alignment */ > - smsc911x_reg_write(pdata, RX_CFG, (2 << 8)); > + smsc911x_reg_write(pdata, RX_CFG, (NET_IP_ALIGN << 8)); > > /* enable NAPI polling before enabling RX interrupts */ > napi_enable(&pdata->napi); Seems fine to me Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 16:08 ` Eric Dumazet @ 2012-04-12 17:03 ` Will Deacon 2012-04-12 17:41 ` Eric Dumazet 2012-04-13 18:08 ` David Miller 0 siblings, 2 replies; 11+ messages in thread From: Will Deacon @ 2012-04-12 17:03 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, Steve Glendinning On Thu, Apr 12, 2012 at 05:08:00PM +0100, Eric Dumazet wrote: > On Thu, 2012-04-12 at 16:54 +0100, Will Deacon wrote: > > net: smsc911x: fix skb handling in receive path > > > > The SMSC911x driver resets the ->head, ->data and ->tail pointers in the > > skb on the reset path in order to avoid buffer overflow due to packet > > padding performed by the hardware. > > > > This patch fixes the receive path so that the skb pointers are fixed up > > after the data has been read from the device, The error path is also > > fixed to use number of words consistently and prevent erroneous FIFO > > fastforwarding when skipping over bad data. > > > > Signed-off-by: Will Deacon <will.deacon@arm.com> [...] > Seems fine to me > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Thanks, Eric. Given that Steve's email doesn't appear to be working, can somebody else pick this up please? Cheers, Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 17:03 ` Will Deacon @ 2012-04-12 17:41 ` Eric Dumazet 2012-04-13 18:08 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: Eric Dumazet @ 2012-04-12 17:41 UTC (permalink / raw) To: Will Deacon, David Miller; +Cc: netdev, Steve Glendinning On Thu, 2012-04-12 at 18:03 +0100, Will Deacon wrote: > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > Thanks, Eric. Given that Steve's email doesn't appear to be working, can > somebody else pick this up please? It seems trivial enough, you tested it, and I reviewed it. I believe David can apply it safely. Thanks ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets 2012-04-12 17:03 ` Will Deacon 2012-04-12 17:41 ` Eric Dumazet @ 2012-04-13 18:08 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: David Miller @ 2012-04-13 18:08 UTC (permalink / raw) To: will.deacon; +Cc: eric.dumazet, netdev, steve.glendinning From: Will Deacon <will.deacon@arm.com> Date: Thu, 12 Apr 2012 18:03:50 +0100 > On Thu, Apr 12, 2012 at 05:08:00PM +0100, Eric Dumazet wrote: >> On Thu, 2012-04-12 at 16:54 +0100, Will Deacon wrote: >> > net: smsc911x: fix skb handling in receive path >> > >> > The SMSC911x driver resets the ->head, ->data and ->tail pointers in the >> > skb on the reset path in order to avoid buffer overflow due to packet >> > padding performed by the hardware. >> > >> > This patch fixes the receive path so that the skb pointers are fixed up >> > after the data has been read from the device, The error path is also >> > fixed to use number of words consistently and prevent erroneous FIFO >> > fastforwarding when skipping over bad data. >> > >> > Signed-off-by: Will Deacon <will.deacon@arm.com> > > [...] > >> Seems fine to me >> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > Thanks, Eric. Given that Steve's email doesn't appear to be working, can > somebody else pick this up please? Applied, thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-04-13 18:08 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-04-12 9:07 [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets Will Deacon 2012-04-12 9:20 ` Eric Dumazet 2012-04-12 12:53 ` Will Deacon 2012-04-12 13:06 ` Eric Dumazet 2012-04-12 13:47 ` Will Deacon 2012-04-12 14:01 ` Eric Dumazet 2012-04-12 15:54 ` Will Deacon 2012-04-12 16:08 ` Eric Dumazet 2012-04-12 17:03 ` Will Deacon 2012-04-12 17:41 ` Eric Dumazet 2012-04-13 18:08 ` David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.