* [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD
@ 2022-09-18 21:55 Sean Anderson
2022-09-19 20:39 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Sean Anderson @ 2022-09-18 21:55 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: Zheyu Ma, open list, Rolf Eike Beer, Nick Bowler, Sean Anderson
There is a separate receive path for small packets (under 256 bytes).
Instead of allocating a new dma-capable skb to be used for the next packet,
this path allocates a skb and copies the data into it (reusing the existing
sbk for the next packet). There are two bytes of junk data at the beginning
of every packet. I believe these are inserted in order to allow aligned
DMA and IP headers. We skip over them using skb_reserve. Before copying
over the data, we must use a barrier to ensure we see the whole packet. The
current code only synchronizes len bytes, starting from the beginning of
the packet, including the junk bytes. However, this leaves off the final
two bytes in the packet. Synchronize the whole packet.
To reproduce this problem, ping a HME with a payload size between 17 and 214
$ ping -s 17 <hme_address>
which will complain rather loudly about the data mismatch. Small packets
(below 60 bytes on the wire) do not have this issue. I suspect this is
related to the padding added to increase the minimum packet size.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
Patch-prefix: net
---
drivers/net/ethernet/sun/sunhme.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index 8594ee839628..88aa0d310aee 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -2020,9 +2020,9 @@ static void happy_meal_rx(struct happy_meal *hp, struct net_device *dev)
skb_reserve(copy_skb, 2);
skb_put(copy_skb, len);
- dma_sync_single_for_cpu(hp->dma_dev, dma_addr, len, DMA_FROM_DEVICE);
+ dma_sync_single_for_cpu(hp->dma_dev, dma_addr, len + 2, DMA_FROM_DEVICE);
skb_copy_from_linear_data(skb, copy_skb->data, len);
- dma_sync_single_for_device(hp->dma_dev, dma_addr, len, DMA_FROM_DEVICE);
+ dma_sync_single_for_device(hp->dma_dev, dma_addr, len + 2, DMA_FROM_DEVICE);
/* Reuse original ring buffer. */
hme_write_rxd(hp, this,
(RXFLAG_OWN|((RX_BUF_ALLOC_SIZE-RX_OFFSET)<<16)),
--
2.37.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD
2022-09-18 21:55 [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD Sean Anderson
@ 2022-09-19 20:39 ` Andrew Lunn
2022-09-20 0:31 ` Sean Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2022-09-19 20:39 UTC (permalink / raw)
To: Sean Anderson
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, Zheyu Ma, open list, Rolf Eike Beer, Nick Bowler
On Sun, Sep 18, 2022 at 05:55:34PM -0400, Sean Anderson wrote:
> There is a separate receive path for small packets (under 256 bytes).
> Instead of allocating a new dma-capable skb to be used for the next packet,
> this path allocates a skb and copies the data into it (reusing the existing
> sbk for the next packet). There are two bytes of junk data at the beginning
> of every packet. I believe these are inserted in order to allow aligned
> DMA and IP headers. We skip over them using skb_reserve. Before copying
> over the data, we must use a barrier to ensure we see the whole packet. The
> current code only synchronizes len bytes, starting from the beginning of
> the packet, including the junk bytes. However, this leaves off the final
> two bytes in the packet. Synchronize the whole packet.
>
> To reproduce this problem, ping a HME with a payload size between 17 and 214
>
> $ ping -s 17 <hme_address>
>
> which will complain rather loudly about the data mismatch. Small packets
> (below 60 bytes on the wire) do not have this issue. I suspect this is
> related to the padding added to increase the minimum packet size.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
Hi Sean
> Patch-prefix: net
This should be in the Subject of the email. Various tools look for the
netdev tree there. Please try to remember that for future patches.
Please could you add a Fixes: tag indicating when the problem was
introduced. Its O.K. if that was when the driver was added. It just
helps getting the patch back ported to older stable kernels.
I think patchwork allows you to just reply to your post, and it will
automagically append the Fixes: tag when the Maintainer actually
applies the patch.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD
2022-09-19 20:39 ` Andrew Lunn
@ 2022-09-20 0:31 ` Sean Anderson
2022-09-20 12:37 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Sean Anderson @ 2022-09-20 0:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, Zheyu Ma, open list, Rolf Eike Beer, Nick Bowler
On 9/19/22 16:39, Andrew Lunn wrote:
> On Sun, Sep 18, 2022 at 05:55:34PM -0400, Sean Anderson wrote:
>> There is a separate receive path for small packets (under 256 bytes).
>> Instead of allocating a new dma-capable skb to be used for the next packet,
>> this path allocates a skb and copies the data into it (reusing the existing
>> sbk for the next packet). There are two bytes of junk data at the beginning
>> of every packet. I believe these are inserted in order to allow aligned
>> DMA and IP headers. We skip over them using skb_reserve. Before copying
>> over the data, we must use a barrier to ensure we see the whole packet. The
>> current code only synchronizes len bytes, starting from the beginning of
>> the packet, including the junk bytes. However, this leaves off the final
>> two bytes in the packet. Synchronize the whole packet.
>>
>> To reproduce this problem, ping a HME with a payload size between 17 and 214
>>
>> $ ping -s 17 <hme_address>
>>
>> which will complain rather loudly about the data mismatch. Small packets
>> (below 60 bytes on the wire) do not have this issue. I suspect this is
>> related to the padding added to increase the minimum packet size.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>
> Hi Sean
>
>> Patch-prefix: net
>
> This should be in the Subject of the email. Various tools look for the
> netdev tree there. Please try to remember that for future patches.
Sorry, it should have been "Series-postfix".
> Please could you add a Fixes: tag indicating when the problem was
> introduced. Its O.K. if that was when the driver was added. It just
> helps getting the patch back ported to older stable kernels.
Well, the driver was added before git was started...
I suppose I could blame 1da177e4c3f4 ("Linux-2.6.12-rc2"), but maybe I
should just CC the stable list?
> I think patchwork allows you to just reply to your post, and it will
> automagically append the Fixes: tag when the Maintainer actually
> applies the patch.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
> Andrew
--Sean
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD
2022-09-20 0:31 ` Sean Anderson
@ 2022-09-20 12:37 ` Andrew Lunn
2022-09-20 13:23 ` Sean Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2022-09-20 12:37 UTC (permalink / raw)
To: Sean Anderson
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, Zheyu Ma, open list, Rolf Eike Beer, Nick Bowler
> > Please could you add a Fixes: tag indicating when the problem was
> > introduced. Its O.K. if that was when the driver was added. It just
> > helps getting the patch back ported to older stable kernels.
>
> Well, the driver was added before git was started...
>
> I suppose I could blame 1da177e4c3f4 ("Linux-2.6.12-rc2"), but maybe I
> should just CC the stable list?
That is a valid commit to use. It is unlikely anybody will backport it
that far, but it does give the machinery a trigger it does need
backporting.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD
2022-09-20 12:37 ` Andrew Lunn
@ 2022-09-20 13:23 ` Sean Anderson
2022-09-20 19:39 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Sean Anderson @ 2022-09-20 13:23 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, Zheyu Ma, open list, Rolf Eike Beer, Nick Bowler
On 9/20/22 08:37, Andrew Lunn wrote:
>>> Please could you add a Fixes: tag indicating when the problem was
>>> introduced. Its O.K. if that was when the driver was added. It just
>>> helps getting the patch back ported to older stable kernels.
>>
>> Well, the driver was added before git was started...
>>
>> I suppose I could blame 1da177e4c3f4 ("Linux-2.6.12-rc2"), but maybe I
>> should just CC the stable list?
>
> That is a valid commit to use. It is unlikely anybody will backport it
> that far, but it does give the machinery a trigger it does need
> backporting.
>
> Andrew
>
OK, well then this
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD
2022-09-20 13:23 ` Sean Anderson
@ 2022-09-20 19:39 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2022-09-20 19:39 UTC (permalink / raw)
To: Sean Anderson
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Paolo Abeni, netdev,
Zheyu Ma, open list, Rolf Eike Beer, Nick Bowler
On Tue, 20 Sep 2022 09:23:09 -0400 Sean Anderson wrote:
> On 9/20/22 08:37, Andrew Lunn wrote:
> >> Well, the driver was added before git was started...
> >>
> >> I suppose I could blame 1da177e4c3f4 ("Linux-2.6.12-rc2"), but maybe I
> >> should just CC the stable list?
> >
> > That is a valid commit to use. It is unlikely anybody will backport it
> > that far, but it does give the machinery a trigger it does need
> > backporting.
>
> OK, well then this
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Please post a v2 with the changes suggested by Andrew folded in.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-20 19:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-18 21:55 [PATCH] net: sunhme: Fix packet reception for len < RX_COPY_THRESHOLD Sean Anderson
2022-09-19 20:39 ` Andrew Lunn
2022-09-20 0:31 ` Sean Anderson
2022-09-20 12:37 ` Andrew Lunn
2022-09-20 13:23 ` Sean Anderson
2022-09-20 19:39 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).