netdev - Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping packets

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120412134757.GH16025@mudshark.cambridge.arm.com>
Date:	Thu, 12 Apr 2012 14:47:57 +0100
From:	Will Deacon <will.deacon@....com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Steve Glendinning <steve.glendinning@...c.com>
Subject: Re: [PATCH] net: smsc911x: fix RX FIFO fastforwarding when dropping
 packets

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
--
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