[net-queue] i40e: Fix incorrect skb reserved size on rx

Message ID 1528877310-2574-1-git-send-email-makita.toshiaki@lab.ntt.co.jp
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series
  • [net-queue] i40e: Fix incorrect skb reserved size on rx
Related show

Commit Message

Toshiaki Makita June 13, 2018, 8:08 a.m.
i40e_build_skb() reserves I40E_SKB_PAD + (xdp->data -
xdp->data_hard_start) but obviously I40E_SKB_PAD is unnecessary here
and mac_header/data feilds in skb becomes incorrect, and breaks normal
skb receive path as well as XDP receive path.

Fixes: cc5b114dcf98 ("bpf, i40e: add meta data support")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Borkmann June 13, 2018, 9:06 a.m. | #1
On 06/13/2018 10:08 AM, Toshiaki Makita wrote:
> i40e_build_skb() reserves I40E_SKB_PAD + (xdp->data -
> xdp->data_hard_start) but obviously I40E_SKB_PAD is unnecessary here
> and mac_header/data feilds in skb becomes incorrect, and breaks normal
> skb receive path as well as XDP receive path.
> 
> Fixes: cc5b114dcf98 ("bpf, i40e: add meta data support")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Thanks Toshiaki, I sent a complete fix yesterday here:

https://lkml.org/lkml/2018/6/12/843

Cheers,
Daniel
Toshiaki Makita June 14, 2018, 12:25 a.m. | #2
On 2018/06/13 18:06, Daniel Borkmann wrote:
> On 06/13/2018 10:08 AM, Toshiaki Makita wrote:
>> i40e_build_skb() reserves I40E_SKB_PAD + (xdp->data -
>> xdp->data_hard_start) but obviously I40E_SKB_PAD is unnecessary here
>> and mac_header/data feilds in skb becomes incorrect, and breaks normal
>> skb receive path as well as XDP receive path.
>>
>> Fixes: cc5b114dcf98 ("bpf, i40e: add meta data support")
>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> 
> Thanks Toshiaki, I sent a complete fix yesterday here:
> 
> https://lkml.org/lkml/2018/6/12/843

Oh I was not aware of it. Thanks, let's go with your patch.
Malek, Patryk June 14, 2018, 9:14 a.m. | #3
> On 2018/06/13 18:06, Daniel Borkmann wrote:
> > On 06/13/2018 10:08 AM, Toshiaki Makita wrote:
> >> i40e_build_skb() reserves I40E_SKB_PAD + (xdp->data -
> >> xdp->data_hard_start) but obviously I40E_SKB_PAD is unnecessary
> here
> >> and mac_header/data feilds in skb becomes incorrect, and breaks

Shouldn't this be fields instead of feilds?

--
Patryk Małek
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
Toshiaki Makita June 14, 2018, 9:21 a.m. | #4
On 2018/06/14 18:14, Malek, Patryk wrote:
>> On 2018/06/13 18:06, Daniel Borkmann wrote:
>>> On 06/13/2018 10:08 AM, Toshiaki Makita wrote:
>>>> i40e_build_skb() reserves I40E_SKB_PAD + (xdp->data -
>>>> xdp->data_hard_start) but obviously I40E_SKB_PAD is unnecessary
>> here
>>>> and mac_header/data feilds in skb becomes incorrect, and breaks
> 
> Shouldn't this be fields instead of feilds?

Thanks, but this is now superseded by Daniel's patch so dropped I think.
http://patchwork.ozlabs.org/patch/928778/

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 8ffb7454e67c..6d59f51f1730 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2124,7 +2124,7 @@  static struct sk_buff *i40e_build_skb(struct i40e_ring *rx_ring,
 		return NULL;
 
 	/* update pointers within the skb to store the data */
-	skb_reserve(skb, I40E_SKB_PAD + (xdp->data - xdp->data_hard_start));
+	skb_reserve(skb, xdp->data - xdp->data_hard_start);
 	__skb_put(skb, xdp->data_end - xdp->data);
 	if (metasize)
 		skb_metadata_set(skb, metasize);