* [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
@ 2022-01-18 10:22 Ard Biesheuvel
2022-01-18 13:14 ` Ilias Apalodimas
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2022-01-18 10:22 UTC (permalink / raw)
To: netdev
Cc: linux-omap, arnd, davem, kuba, Ard Biesheuvel, Grygorii Strashko,
Ilias Apalodimas
Both versions of the CPSW driver declare a CPSW_HEADROOM_NA macro that
takes NET_IP_ALIGN into account, but fail to use it appropriately when
storing incoming packets in memory. This results in the IPv4 source and
destination addresses to appear misaligned in memory, which causes
aligment faults that need to be fixed up in software.
So let's switch from CPSW_HEADROOM to CPSW_HEADROOM_NA where needed.
This gets rid of any alignment faults on the RX path on a Beaglebone
White.
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 6 +++---
drivers/net/ethernet/ti/cpsw_new.c | 6 +++---
drivers/net/ethernet/ti/cpsw_priv.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 33142d505fc8..03575c017500 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -349,7 +349,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
struct cpsw_common *cpsw = ndev_to_cpsw(xmeta->ndev);
int pkt_size = cpsw->rx_packet_max;
int ret = 0, port, ch = xmeta->ch;
- int headroom = CPSW_HEADROOM;
+ int headroom = CPSW_HEADROOM_NA;
struct net_device *ndev = xmeta->ndev;
struct cpsw_priv *priv;
struct page_pool *pool;
@@ -392,7 +392,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
}
if (priv->xdp_prog) {
- int headroom = CPSW_HEADROOM, size = len;
+ int size = len;
xdp_init_buff(&xdp, PAGE_SIZE, &priv->xdp_rxq[ch]);
if (status & CPDMA_RX_VLAN_ENCAP) {
@@ -442,7 +442,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
xmeta->ndev = ndev;
xmeta->ch = ch;
- dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM;
+ dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM_NA;
ret = cpdma_chan_submit_mapped(cpsw->rxv[ch].ch, new_page, dma,
pkt_size, 0);
if (ret < 0) {
diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index 279e261e4720..bd4b1528cf99 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -283,7 +283,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
{
struct page *new_page, *page = token;
void *pa = page_address(page);
- int headroom = CPSW_HEADROOM;
+ int headroom = CPSW_HEADROOM_NA;
struct cpsw_meta_xdp *xmeta;
struct cpsw_common *cpsw;
struct net_device *ndev;
@@ -336,7 +336,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
}
if (priv->xdp_prog) {
- int headroom = CPSW_HEADROOM, size = len;
+ int size = len;
xdp_init_buff(&xdp, PAGE_SIZE, &priv->xdp_rxq[ch]);
if (status & CPDMA_RX_VLAN_ENCAP) {
@@ -386,7 +386,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
xmeta->ndev = ndev;
xmeta->ch = ch;
- dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM;
+ dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM_NA;
ret = cpdma_chan_submit_mapped(cpsw->rxv[ch].ch, new_page, dma,
pkt_size, 0);
if (ret < 0) {
diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
index 3537502e5e8b..ba220593e6db 100644
--- a/drivers/net/ethernet/ti/cpsw_priv.c
+++ b/drivers/net/ethernet/ti/cpsw_priv.c
@@ -1122,7 +1122,7 @@ int cpsw_fill_rx_channels(struct cpsw_priv *priv)
xmeta->ndev = priv->ndev;
xmeta->ch = ch;
- dma = page_pool_get_dma_addr(page) + CPSW_HEADROOM;
+ dma = page_pool_get_dma_addr(page) + CPSW_HEADROOM_NA;
ret = cpdma_chan_idle_submit_mapped(cpsw->rxv[ch].ch,
page, dma,
cpsw->rx_packet_max,
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
2022-01-18 10:22 [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account Ard Biesheuvel
@ 2022-01-18 13:14 ` Ilias Apalodimas
2022-01-18 21:03 ` Jakub Kicinski
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2022-01-18 13:14 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: netdev, linux-omap, arnd, davem, kuba, Grygorii Strashko
Hi Ard,
On Tue, 18 Jan 2022 at 12:22, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Both versions of the CPSW driver declare a CPSW_HEADROOM_NA macro that
> takes NET_IP_ALIGN into account, but fail to use it appropriately when
> storing incoming packets in memory. This results in the IPv4 source and
> destination addresses to appear misaligned in memory, which causes
> aligment faults that need to be fixed up in software.
>
> So let's switch from CPSW_HEADROOM to CPSW_HEADROOM_NA where needed.
> This gets rid of any alignment faults on the RX path on a Beaglebone
> White.
>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> drivers/net/ethernet/ti/cpsw.c | 6 +++---
> drivers/net/ethernet/ti/cpsw_new.c | 6 +++---
> drivers/net/ethernet/ti/cpsw_priv.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 33142d505fc8..03575c017500 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -349,7 +349,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
> struct cpsw_common *cpsw = ndev_to_cpsw(xmeta->ndev);
> int pkt_size = cpsw->rx_packet_max;
> int ret = 0, port, ch = xmeta->ch;
> - int headroom = CPSW_HEADROOM;
> + int headroom = CPSW_HEADROOM_NA;
> struct net_device *ndev = xmeta->ndev;
> struct cpsw_priv *priv;
> struct page_pool *pool;
> @@ -392,7 +392,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
> }
>
> if (priv->xdp_prog) {
> - int headroom = CPSW_HEADROOM, size = len;
> + int size = len;
>
> xdp_init_buff(&xdp, PAGE_SIZE, &priv->xdp_rxq[ch]);
> if (status & CPDMA_RX_VLAN_ENCAP) {
> @@ -442,7 +442,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
> xmeta->ndev = ndev;
> xmeta->ch = ch;
>
> - dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM;
> + dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM_NA;
> ret = cpdma_chan_submit_mapped(cpsw->rxv[ch].ch, new_page, dma,
> pkt_size, 0);
> if (ret < 0) {
> diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
> index 279e261e4720..bd4b1528cf99 100644
> --- a/drivers/net/ethernet/ti/cpsw_new.c
> +++ b/drivers/net/ethernet/ti/cpsw_new.c
> @@ -283,7 +283,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
> {
> struct page *new_page, *page = token;
> void *pa = page_address(page);
> - int headroom = CPSW_HEADROOM;
> + int headroom = CPSW_HEADROOM_NA;
> struct cpsw_meta_xdp *xmeta;
> struct cpsw_common *cpsw;
> struct net_device *ndev;
> @@ -336,7 +336,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
> }
>
> if (priv->xdp_prog) {
> - int headroom = CPSW_HEADROOM, size = len;
> + int size = len;
>
> xdp_init_buff(&xdp, PAGE_SIZE, &priv->xdp_rxq[ch]);
> if (status & CPDMA_RX_VLAN_ENCAP) {
> @@ -386,7 +386,7 @@ static void cpsw_rx_handler(void *token, int len, int status)
> xmeta->ndev = ndev;
> xmeta->ch = ch;
>
> - dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM;
> + dma = page_pool_get_dma_addr(new_page) + CPSW_HEADROOM_NA;
> ret = cpdma_chan_submit_mapped(cpsw->rxv[ch].ch, new_page, dma,
> pkt_size, 0);
> if (ret < 0) {
> diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
> index 3537502e5e8b..ba220593e6db 100644
> --- a/drivers/net/ethernet/ti/cpsw_priv.c
> +++ b/drivers/net/ethernet/ti/cpsw_priv.c
> @@ -1122,7 +1122,7 @@ int cpsw_fill_rx_channels(struct cpsw_priv *priv)
> xmeta->ndev = priv->ndev;
> xmeta->ch = ch;
>
> - dma = page_pool_get_dma_addr(page) + CPSW_HEADROOM;
> + dma = page_pool_get_dma_addr(page) + CPSW_HEADROOM_NA;
> ret = cpdma_chan_idle_submit_mapped(cpsw->rxv[ch].ch,
> page, dma,
> cpsw->rx_packet_max,
> --
> 2.30.2
>
This looks good to me.
Grygorii I assume cpdma is ok with potential unaligned accesses?
Thanks
/Ilias
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
2022-01-18 10:22 [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account Ard Biesheuvel
2022-01-18 13:14 ` Ilias Apalodimas
@ 2022-01-18 21:03 ` Jakub Kicinski
2022-01-18 21:32 ` Ard Biesheuvel
2022-01-19 14:20 ` patchwork-bot+netdevbpf
2022-01-19 14:25 ` Alexey Smirnov
3 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2022-01-18 21:03 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: netdev, linux-omap, arnd, davem, Grygorii Strashko, Ilias Apalodimas
On Tue, 18 Jan 2022 11:22:04 +0100 Ard Biesheuvel wrote:
> Both versions of the CPSW driver declare a CPSW_HEADROOM_NA macro that
> takes NET_IP_ALIGN into account, but fail to use it appropriately when
> storing incoming packets in memory. This results in the IPv4 source and
> destination addresses to appear misaligned in memory, which causes
> aligment faults that need to be fixed up in software.
>
> So let's switch from CPSW_HEADROOM to CPSW_HEADROOM_NA where needed.
> This gets rid of any alignment faults on the RX path on a Beaglebone
> White.
>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Fixes: 9ed4050c0d75 ("net: ethernet: ti: cpsw: add XDP support")
right?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
2022-01-18 21:03 ` Jakub Kicinski
@ 2022-01-18 21:32 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2022-01-18 21:32 UTC (permalink / raw)
To: Jakub Kicinski
Cc: open list:BPF JIT for MIPS (32-BIT AND 64-BIT),
linux-omap, Arnd Bergmann, David S. Miller, Grygorii Strashko,
Ilias Apalodimas
On Tue, 18 Jan 2022 at 22:03, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 18 Jan 2022 11:22:04 +0100 Ard Biesheuvel wrote:
> > Both versions of the CPSW driver declare a CPSW_HEADROOM_NA macro that
> > takes NET_IP_ALIGN into account, but fail to use it appropriately when
> > storing incoming packets in memory. This results in the IPv4 source and
> > destination addresses to appear misaligned in memory, which causes
> > aligment faults that need to be fixed up in software.
> >
> > So let's switch from CPSW_HEADROOM to CPSW_HEADROOM_NA where needed.
> > This gets rid of any alignment faults on the RX path on a Beaglebone
> > White.
> >
> > Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Fixes: 9ed4050c0d75 ("net: ethernet: ti: cpsw: add XDP support")
>
I suspect so, as that patch removes a call to
__netdev_alloc_skb_ip_align(), and replaces it with
page_pool_dev_alloc_pages() et al
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
2022-01-18 10:22 [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account Ard Biesheuvel
2022-01-18 13:14 ` Ilias Apalodimas
2022-01-18 21:03 ` Jakub Kicinski
@ 2022-01-19 14:20 ` patchwork-bot+netdevbpf
2022-01-19 14:25 ` Alexey Smirnov
3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-01-19 14:20 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: netdev, linux-omap, arnd, davem, kuba, grygorii.strashko,
ilias.apalodimas
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Tue, 18 Jan 2022 11:22:04 +0100 you wrote:
> Both versions of the CPSW driver declare a CPSW_HEADROOM_NA macro that
> takes NET_IP_ALIGN into account, but fail to use it appropriately when
> storing incoming packets in memory. This results in the IPv4 source and
> destination addresses to appear misaligned in memory, which causes
> aligment faults that need to be fixed up in software.
>
> So let's switch from CPSW_HEADROOM to CPSW_HEADROOM_NA where needed.
> This gets rid of any alignment faults on the RX path on a Beaglebone
> White.
>
> [...]
Here is the summary with links:
- [net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
https://git.kernel.org/netdev/net/c/1771afd47430
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
2022-01-18 10:22 [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account Ard Biesheuvel
` (2 preceding siblings ...)
2022-01-19 14:20 ` patchwork-bot+netdevbpf
@ 2022-01-19 14:25 ` Alexey Smirnov
2022-01-19 15:56 ` Ard Biesheuvel
3 siblings, 1 reply; 7+ messages in thread
From: Alexey Smirnov @ 2022-01-19 14:25 UTC (permalink / raw)
To: ardb
Cc: arnd, davem, grygorii.strashko, ilias.apalodimas, kuba,
linux-omap, netdev
Doesn't CPSW_HEADROOM already include NET_IP_ALIGN and has actually more strict
alignment (by sizeof(long)) than CPSW_HEADROOM_NA?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account
2022-01-19 14:25 ` Alexey Smirnov
@ 2022-01-19 15:56 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2022-01-19 15:56 UTC (permalink / raw)
To: Alexey Smirnov
Cc: Arnd Bergmann, David S. Miller, Grygorii Strashko,
Ilias Apalodimas, Jakub Kicinski, linux-omap,
open list:BPF JIT for MIPS (32-BIT AND 64-BIT)
On Wed, 19 Jan 2022 at 15:25, Alexey Smirnov <s.alexey@gmail.com> wrote:
>
> Doesn't CPSW_HEADROOM already include NET_IP_ALIGN and has actually more strict
> alignment (by sizeof(long)) than CPSW_HEADROOM_NA?
Yes, and that is exactly the problem. NET_IP_ALIGN is used to
deliberately *misalign* the start of the packet in memory, so that the
IP header appears aligned.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-19 15:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-18 10:22 [PATCH net] net: cpsw: avoid alignment faults by taking NET_IP_ALIGN into account Ard Biesheuvel
2022-01-18 13:14 ` Ilias Apalodimas
2022-01-18 21:03 ` Jakub Kicinski
2022-01-18 21:32 ` Ard Biesheuvel
2022-01-19 14:20 ` patchwork-bot+netdevbpf
2022-01-19 14:25 ` Alexey Smirnov
2022-01-19 15:56 ` Ard Biesheuvel
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.