netdev - Re: [PATCH][net-next] gianfar: Fix reported number of sent bytes to BQL

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: <1943289.AeBmeBuHV2@lenovo>
Date:	Tue, 03 Sep 2013 20:18:12 +0100
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Claudiu Manoil <claudiu.manoil@...escale.com>
Cc:	netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [PATCH][net-next] gianfar: Fix reported number of sent bytes to BQL

Le mardi 3 septembre 2013 19:59:42 Florian Fainelli a écrit :
> Hello Claudiu,
> 
> 2013/8/30 Claudiu Manoil <claudiu.manoil@...escale.com>:
> > Fix the amount of sent bytes reported to BQL by reporting the
> > number of bytes on wire in the xmit routine, and recording that
> > value for each skb in order to be correctly confirmed on Tx
> > confirmation cleanup.
> > 
> > Reporting skb->len to BQL just before exiting xmit is not correct
> > due to possible insertions of TOE block and alignment bytes in the
> > skb->data, which are being stripped off by the controller before
> > transmission on wire.  This led to mismatch of (incorrectly)
> > reported bytes to BQL b/w xmit and Tx confirmation, resulting in
> > Tx timeout firing, for the h/w tx timestamping acceleration case.
> > 
> > There's no easy way to obtain the number of bytes on wire in the Tx
> > confirmation routine, so skb->cb is used to convey that information
> > from xmit to Tx confirmation, for now (as proposed by Eric). Revived
> > the currently unused GFAR_CB() construct for that purpose.
> 
> I do not see much difference between what this patch does and what the
> current net-next drivers does. If you need to correctly account for
> this, it seems to me like you should move the stats/bytes_sent
> computation below this line:
> 
>        if (unlikely(do_tstamp)) {
>                 skb_push(skb, GMAC_TXPAL_LEN);
>                 memset(skb->data, 0, GMAC_TXPAL_LEN);
>         }
> 
> to account for the SKB length update?

Just realized that this is precisely what your patch is fixing, sorry for the 
noise.
-- 
Florian
--
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