All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
@ 2021-01-05 17:19 Marek Behún
  2021-01-05 17:24 ` Sven Auhagen
  2021-01-05 20:23 ` Andrew Lunn
  0 siblings, 2 replies; 13+ messages in thread
From: Marek Behún @ 2021-01-05 17:19 UTC (permalink / raw)
  To: netdev
  Cc: davem, Jakub Kicinski, Marek Behún, Sven Auhagen,
	Matteo Croce, Lorenzo Bianconi

Currently mvpp2_xdp_setup won't allow attaching XDP program if
  mtu > ETH_DATA_LEN (1500).

The mvpp2_change_mtu on the other hand checks whether
  MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE.

These two checks are semantically different.

Moreover this limit can be increased to MVPP2_MAX_RX_BUF_SIZE, since in
mvpp2_rx we have
  xdp.data = data + MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
  xdp.frame_sz = PAGE_SIZE;

Change the checks to check whether
  mtu > MVPP2_MAX_RX_BUF_SIZE

Signed-off-by: Marek Behún <kabel@kernel.org>
Cc: Sven Auhagen <sven.auhagen@voleatech.de>
Cc: Matteo Croce <mcroce@microsoft.com>
Cc: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index afdd22827223..65490a0eb657 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4623,11 +4623,12 @@ static int mvpp2_change_mtu(struct net_device *dev, int mtu)
 		mtu = ALIGN(MVPP2_RX_PKT_SIZE(mtu), 8);
 	}
 
+	if (port->xdp_prog && mtu > MVPP2_MAX_RX_BUF_SIZE) {
+		netdev_err(dev, "Illegal MTU value %d for XDP mode\n", mtu);
+		return -EINVAL;
+	}
+
 	if (MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE) {
-		if (port->xdp_prog) {
-			netdev_err(dev, "Jumbo frames are not supported with XDP\n");
-			return -EINVAL;
-		}
 		if (priv->percpu_pools) {
 			netdev_warn(dev, "mtu %d too high, switching to shared buffers", mtu);
 			mvpp2_bm_switch_buffers(priv, false);
@@ -4913,8 +4914,8 @@ static int mvpp2_xdp_setup(struct mvpp2_port *port, struct netdev_bpf *bpf)
 	bool running = netif_running(port->dev);
 	bool reset = !prog != !port->xdp_prog;
 
-	if (port->dev->mtu > ETH_DATA_LEN) {
-		NL_SET_ERR_MSG_MOD(bpf->extack, "XDP is not supported with jumbo frames enabled");
+	if (port->dev->mtu > MVPP2_MAX_RX_BUF_SIZE) {
+		NL_SET_ERR_MSG_MOD(bpf->extack, "MTU too large for XDP");
 		return -EOPNOTSUPP;
 	}
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-05 17:19 [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled Marek Behún
@ 2021-01-05 17:24 ` Sven Auhagen
  2021-01-05 17:43   ` Marek Behún
  2021-01-05 20:23 ` Andrew Lunn
  1 sibling, 1 reply; 13+ messages in thread
From: Sven Auhagen @ 2021-01-05 17:24 UTC (permalink / raw)
  To: Marek Behún
  Cc: netdev, davem, Jakub Kicinski, Matteo Croce, Lorenzo Bianconi

On Tue, Jan 05, 2021 at 06:19:21PM +0100, Marek Behún wrote:
> Currently mvpp2_xdp_setup won't allow attaching XDP program if
>   mtu > ETH_DATA_LEN (1500).
> 
> The mvpp2_change_mtu on the other hand checks whether
>   MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE.
> 
> These two checks are semantically different.
> 
> Moreover this limit can be increased to MVPP2_MAX_RX_BUF_SIZE, since in
> mvpp2_rx we have
>   xdp.data = data + MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
>   xdp.frame_sz = PAGE_SIZE;
> 
> Change the checks to check whether
>   mtu > MVPP2_MAX_RX_BUF_SIZE

Hello Marek,

in general, XDP is based on the model, that packets are not bigger than 1500.
I am not sure if that has changed, I don't believe Jumbo Frames are upstreamed yet.
You are correct that the MVPP2 driver can handle bigger packets without a problem but
if you do XDP redirect that won't work with other drivers and your packets will disappear.

Best
Sven

> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> Cc: Sven Auhagen <sven.auhagen@voleatech.de>
> Cc: Matteo Croce <mcroce@microsoft.com>
> Cc: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index afdd22827223..65490a0eb657 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4623,11 +4623,12 @@ static int mvpp2_change_mtu(struct net_device *dev, int mtu)
>  		mtu = ALIGN(MVPP2_RX_PKT_SIZE(mtu), 8);
>  	}
>  
> +	if (port->xdp_prog && mtu > MVPP2_MAX_RX_BUF_SIZE) {
> +		netdev_err(dev, "Illegal MTU value %d for XDP mode\n", mtu);
> +		return -EINVAL;
> +	}
> +
>  	if (MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE) {
> -		if (port->xdp_prog) {
> -			netdev_err(dev, "Jumbo frames are not supported with XDP\n");
> -			return -EINVAL;
> -		}
>  		if (priv->percpu_pools) {
>  			netdev_warn(dev, "mtu %d too high, switching to shared buffers", mtu);
>  			mvpp2_bm_switch_buffers(priv, false);
> @@ -4913,8 +4914,8 @@ static int mvpp2_xdp_setup(struct mvpp2_port *port, struct netdev_bpf *bpf)
>  	bool running = netif_running(port->dev);
>  	bool reset = !prog != !port->xdp_prog;
>  
> -	if (port->dev->mtu > ETH_DATA_LEN) {
> -		NL_SET_ERR_MSG_MOD(bpf->extack, "XDP is not supported with jumbo frames enabled");
> +	if (port->dev->mtu > MVPP2_MAX_RX_BUF_SIZE) {
> +		NL_SET_ERR_MSG_MOD(bpf->extack, "MTU too large for XDP");
>  		return -EOPNOTSUPP;
>  	}
>  
> -- 
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-05 17:24 ` Sven Auhagen
@ 2021-01-05 17:43   ` Marek Behún
  2021-01-05 20:21     ` Andrew Lunn
  2021-01-06 10:33     ` Jesper Dangaard Brouer
  0 siblings, 2 replies; 13+ messages in thread
From: Marek Behún @ 2021-01-05 17:43 UTC (permalink / raw)
  To: Sven Auhagen
  Cc: netdev, davem, Jakub Kicinski, Matteo Croce, Lorenzo Bianconi

On Tue, 5 Jan 2021 18:24:37 +0100
Sven Auhagen <sven.auhagen@voleatech.de> wrote:

> On Tue, Jan 05, 2021 at 06:19:21PM +0100, Marek Behún wrote:
> > Currently mvpp2_xdp_setup won't allow attaching XDP program if
> >   mtu > ETH_DATA_LEN (1500).
> > 
> > The mvpp2_change_mtu on the other hand checks whether
> >   MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE.
> > 
> > These two checks are semantically different.
> > 
> > Moreover this limit can be increased to MVPP2_MAX_RX_BUF_SIZE, since in
> > mvpp2_rx we have
> >   xdp.data = data + MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
> >   xdp.frame_sz = PAGE_SIZE;
> > 
> > Change the checks to check whether
> >   mtu > MVPP2_MAX_RX_BUF_SIZE  
> 
> Hello Marek,
> 
> in general, XDP is based on the model, that packets are not bigger than 1500.
> I am not sure if that has changed, I don't believe Jumbo Frames are upstreamed yet.
> You are correct that the MVPP2 driver can handle bigger packets without a problem but
> if you do XDP redirect that won't work with other drivers and your packets will disappear.

At least 1508 is required when I want to use XDP with a Marvell DSA
switch: the DSA header is 4 or 8 bytes long there.

The DSA driver increases MTU on CPU switch interface by this length
(on my switches to 1504).

So without this I cannot use XDP with mvpp2 with a Marvell switch with
default settings, which I think is not OK.

Since with the mvneta driver it works (mvneta checks for
MVNETA_MAX_RX_BUF_SIZE rather than ETH_DATA_LEN), I think it should also work
with mvpp2.

Marek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-05 17:43   ` Marek Behún
@ 2021-01-05 20:21     ` Andrew Lunn
  2021-01-06 11:56       ` Marek Behún
  2021-01-06 10:33     ` Jesper Dangaard Brouer
  1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2021-01-05 20:21 UTC (permalink / raw)
  To: Marek Behún
  Cc: Sven Auhagen, netdev, davem, Jakub Kicinski, Matteo Croce,
	Lorenzo Bianconi

On Tue, Jan 05, 2021 at 06:43:08PM +0100, Marek Behún wrote:
> On Tue, 5 Jan 2021 18:24:37 +0100
> Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> 
> > On Tue, Jan 05, 2021 at 06:19:21PM +0100, Marek Behún wrote:
> > > Currently mvpp2_xdp_setup won't allow attaching XDP program if
> > >   mtu > ETH_DATA_LEN (1500).
> > > 
> > > The mvpp2_change_mtu on the other hand checks whether
> > >   MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE.
> > > 
> > > These two checks are semantically different.
> > > 
> > > Moreover this limit can be increased to MVPP2_MAX_RX_BUF_SIZE, since in
> > > mvpp2_rx we have
> > >   xdp.data = data + MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
> > >   xdp.frame_sz = PAGE_SIZE;
> > > 
> > > Change the checks to check whether
> > >   mtu > MVPP2_MAX_RX_BUF_SIZE  
> > 
> > Hello Marek,
> > 
> > in general, XDP is based on the model, that packets are not bigger than 1500.
> > I am not sure if that has changed, I don't believe Jumbo Frames are upstreamed yet.
> > You are correct that the MVPP2 driver can handle bigger packets without a problem but
> > if you do XDP redirect that won't work with other drivers and your packets will disappear.
> 
> At least 1508 is required when I want to use XDP with a Marvell DSA
> switch: the DSA header is 4 or 8 bytes long there.
> 
> The DSA driver increases MTU on CPU switch interface by this length
> (on my switches to 1504).
> 
> So without this I cannot use XDP with mvpp2 with a Marvell switch with
> default settings, which I think is not OK.

Hi Marek

You are running XDP programs on the master interface? So you still
have the DSA tag? What sort of programs are you running? I guess DOS
protection could work, once the program understands the DSA tag. To
forward the frame out another interface you would have to remove the
tag. Or i suppose you could modify the tag and send it back to the
switch? Does XDP support that sort of hairpin operation?

	Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-05 17:19 [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled Marek Behún
  2021-01-05 17:24 ` Sven Auhagen
@ 2021-01-05 20:23 ` Andrew Lunn
  2021-01-06 12:11   ` Marek Behún
  1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2021-01-05 20:23 UTC (permalink / raw)
  To: Marek Behún
  Cc: netdev, davem, Jakub Kicinski, Sven Auhagen, Matteo Croce,
	Lorenzo Bianconi

> @@ -4913,8 +4914,8 @@ static int mvpp2_xdp_setup(struct mvpp2_port *port, struct netdev_bpf *bpf)
>  	bool running = netif_running(port->dev);
>  	bool reset = !prog != !port->xdp_prog;
>  
> -	if (port->dev->mtu > ETH_DATA_LEN) {
> -		NL_SET_ERR_MSG_MOD(bpf->extack, "XDP is not supported with jumbo frames enabled");
> +	if (port->dev->mtu > MVPP2_MAX_RX_BUF_SIZE) {
> +		NL_SET_ERR_MSG_MOD(bpf->extack, "MTU too large for XDP");

Hi Marek

Since MVPP2_MAX_RX_BUF_SIZE is a constant, you can probably do some
CPP magic and have it part of the extack message, to give the user a
clue what the maximum is.

     Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-05 17:43   ` Marek Behún
  2021-01-05 20:21     ` Andrew Lunn
@ 2021-01-06 10:33     ` Jesper Dangaard Brouer
  2021-01-06 11:28       ` Sven Auhagen
  1 sibling, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2021-01-06 10:33 UTC (permalink / raw)
  To: Marek Behún
  Cc: brouer, Sven Auhagen, netdev, davem, Jakub Kicinski,
	Matteo Croce, Lorenzo Bianconi, John Fastabend

On Tue, 5 Jan 2021 18:43:08 +0100
Marek Behún <kabel@kernel.org> wrote:

> On Tue, 5 Jan 2021 18:24:37 +0100
> Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> 
> > On Tue, Jan 05, 2021 at 06:19:21PM +0100, Marek Behún wrote:  
> > > Currently mvpp2_xdp_setup won't allow attaching XDP program if
> > >   mtu > ETH_DATA_LEN (1500).
> > > 
> > > The mvpp2_change_mtu on the other hand checks whether
> > >   MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE.
> > > 
> > > These two checks are semantically different.
> > > 
> > > Moreover this limit can be increased to MVPP2_MAX_RX_BUF_SIZE, since in
> > > mvpp2_rx we have
> > >   xdp.data = data + MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
> > >   xdp.frame_sz = PAGE_SIZE;
> > > 
> > > Change the checks to check whether
> > >   mtu > MVPP2_MAX_RX_BUF_SIZE    
> > 
> > Hello Marek,
> > 
> > in general, XDP is based on the model, that packets are not bigger
> > than 1500.

This is WRONG.

The XDP design/model (with PAGE_SIZE 4096) allows MTU to be 3506 bytes.

This comes from:
 * 4096 = frame_sz = PAGE_SIZE
 * -256 = reserved XDP_PACKET_HEADROOM
 * -320 = reserved tailroom with sizeof(skb_shared_info)
 * - 14 = Ethernet header size as MTU value is L3

4096-256-320-14 = 3506 bytes

Depending on driver memory layout choices this can (of-cause) be lower.

> > I am not sure if that has changed, I don't believe Jumbo Frames are
> > upstreamed yet.

This is unrelated to this patch, but Lorenzo and Eelco is assigned to
work on this.

> > You are correct that the MVPP2 driver can handle bigger packets
> > without a problem but if you do XDP redirect that won't work with
> > other drivers and your packets will disappear.  
> 

This statement is too harsh.  The XDP layer will not do (IP-level)
fragmentation for you.  Thus, if you redirect/transmit frames out
another interface with lower MTU than the frame packet size then the
packet will of-cause be dropped (the drop counter is unfortunately not
well defined).  This is pretty standard behavior.

This is why I'm proposing the BPF-helper bpf_check_mtu().  To allow the
BPF-prog to check the MTU before doing the redirect.


> At least 1508 is required when I want to use XDP with a Marvell DSA
> switch: the DSA header is 4 or 8 bytes long there.
> 
> The DSA driver increases MTU on CPU switch interface by this length
> (on my switches to 1504).
> 
> So without this I cannot use XDP with mvpp2 with a Marvell switch with
> default settings, which I think is not OK.
> 
> Since with the mvneta driver it works (mvneta checks for
> MVNETA_MAX_RX_BUF_SIZE rather than ETH_DATA_LEN), I think it should also work
> with mvpp2.

I think you patch makes perfect sense.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-06 10:33     ` Jesper Dangaard Brouer
@ 2021-01-06 11:28       ` Sven Auhagen
  0 siblings, 0 replies; 13+ messages in thread
From: Sven Auhagen @ 2021-01-06 11:28 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Marek Behún, netdev, davem, Jakub Kicinski, Matteo Croce,
	Lorenzo Bianconi, John Fastabend

On Wed, Jan 06, 2021 at 11:33:50AM +0100, Jesper Dangaard Brouer wrote:
> On Tue, 5 Jan 2021 18:43:08 +0100
> Marek Behún <kabel@kernel.org> wrote:
> 
> > On Tue, 5 Jan 2021 18:24:37 +0100
> > Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > 
> > > On Tue, Jan 05, 2021 at 06:19:21PM +0100, Marek Behún wrote:  
> > > > Currently mvpp2_xdp_setup won't allow attaching XDP program if
> > > >   mtu > ETH_DATA_LEN (1500).
> > > > 
> > > > The mvpp2_change_mtu on the other hand checks whether
> > > >   MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE.
> > > > 
> > > > These two checks are semantically different.
> > > > 
> > > > Moreover this limit can be increased to MVPP2_MAX_RX_BUF_SIZE, since in
> > > > mvpp2_rx we have
> > > >   xdp.data = data + MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
> > > >   xdp.frame_sz = PAGE_SIZE;
> > > > 
> > > > Change the checks to check whether
> > > >   mtu > MVPP2_MAX_RX_BUF_SIZE    
> > > 
> > > Hello Marek,
> > > 
> > > in general, XDP is based on the model, that packets are not bigger
> > > than 1500.
> 
> This is WRONG.
> 
> The XDP design/model (with PAGE_SIZE 4096) allows MTU to be 3506 bytes.
> 
> This comes from:
>  * 4096 = frame_sz = PAGE_SIZE
>  * -256 = reserved XDP_PACKET_HEADROOM
>  * -320 = reserved tailroom with sizeof(skb_shared_info)
>  * - 14 = Ethernet header size as MTU value is L3
> 
> 4096-256-320-14 = 3506 bytes
> 
> Depending on driver memory layout choices this can (of-cause) be lower.

Got it, thanks.

> 
> > > I am not sure if that has changed, I don't believe Jumbo Frames are
> > > upstreamed yet.
> 
> This is unrelated to this patch, but Lorenzo and Eelco is assigned to
> work on this.
> 
> > > You are correct that the MVPP2 driver can handle bigger packets
> > > without a problem but if you do XDP redirect that won't work with
> > > other drivers and your packets will disappear.  
> > 
> 
> This statement is too harsh.  The XDP layer will not do (IP-level)
> fragmentation for you.  Thus, if you redirect/transmit frames out
> another interface with lower MTU than the frame packet size then the
> packet will of-cause be dropped (the drop counter is unfortunately not
> well defined).  This is pretty standard behavior.

Some drivers do not have a XDP drop counter and from own testing it is very difficult
to find out what happened to the packet when it is dropped like that.

> 
> This is why I'm proposing the BPF-helper bpf_check_mtu().  To allow the
> BPF-prog to check the MTU before doing the redirect.
> 
> 
> > At least 1508 is required when I want to use XDP with a Marvell DSA
> > switch: the DSA header is 4 or 8 bytes long there.
> > 
> > The DSA driver increases MTU on CPU switch interface by this length
> > (on my switches to 1504).
> > 
> > So without this I cannot use XDP with mvpp2 with a Marvell switch with
> > default settings, which I think is not OK.
> > 
> > Since with the mvneta driver it works (mvneta checks for
> > MVNETA_MAX_RX_BUF_SIZE rather than ETH_DATA_LEN), I think it should also work
> > with mvpp2.
> 
> I think you patch makes perfect sense.
> 
> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.linkedin.com%2Fin%2Fbrouer&amp;data=04%7C01%7Csven.auhagen%40voleatech.de%7C2450996aa72245f4a6da08d8b22e971a%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637455260465184088%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LDmq8nFGgKuzG3rbqmaILTw6W4Qsc04MULSQvwmoVLw%3D&amp;reserved=0
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-05 20:21     ` Andrew Lunn
@ 2021-01-06 11:56       ` Marek Behún
  2021-01-06 12:32         ` Marek Behún
  2021-01-06 13:13         ` Vladimir Oltean
  0 siblings, 2 replies; 13+ messages in thread
From: Marek Behún @ 2021-01-06 11:56 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Sven Auhagen, netdev, davem, Jakub Kicinski, Matteo Croce,
	Lorenzo Bianconi

On Tue, 5 Jan 2021 21:21:10 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Tue, Jan 05, 2021 at 06:43:08PM +0100, Marek Behún wrote:
> > On Tue, 5 Jan 2021 18:24:37 +0100
> > Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> >   
> > > On Tue, Jan 05, 2021 at 06:19:21PM +0100, Marek Behún wrote:  
> > > > Currently mvpp2_xdp_setup won't allow attaching XDP program if
> > > >   mtu > ETH_DATA_LEN (1500).
> > > > 
> > > > The mvpp2_change_mtu on the other hand checks whether
> > > >   MVPP2_RX_PKT_SIZE(mtu) > MVPP2_BM_LONG_PKT_SIZE.
> > > > 
> > > > These two checks are semantically different.
> > > > 
> > > > Moreover this limit can be increased to MVPP2_MAX_RX_BUF_SIZE, since in
> > > > mvpp2_rx we have
> > > >   xdp.data = data + MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM;
> > > >   xdp.frame_sz = PAGE_SIZE;
> > > > 
> > > > Change the checks to check whether
> > > >   mtu > MVPP2_MAX_RX_BUF_SIZE    
> > > 
> > > Hello Marek,
> > > 
> > > in general, XDP is based on the model, that packets are not bigger than 1500.
> > > I am not sure if that has changed, I don't believe Jumbo Frames are upstreamed yet.
> > > You are correct that the MVPP2 driver can handle bigger packets without a problem but
> > > if you do XDP redirect that won't work with other drivers and your packets will disappear.  
> > 
> > At least 1508 is required when I want to use XDP with a Marvell DSA
> > switch: the DSA header is 4 or 8 bytes long there.
> > 
> > The DSA driver increases MTU on CPU switch interface by this length
> > (on my switches to 1504).
> > 
> > So without this I cannot use XDP with mvpp2 with a Marvell switch with
> > default settings, which I think is not OK.  
> 
> Hi Marek
> 
> You are running XDP programs on the master interface? So you still
> have the DSA tag? What sort of programs are you running? I guess DOS
> protection could work, once the program understands the DSA tag. To
> forward the frame out another interface you would have to remove the
> tag. Or i suppose you could modify the tag and send it back to the
> switch? Does XDP support that sort of hairpin operation?
> 
> 	Andrew

Andrew,

I am using bpf_fib_lookup to route the packets between switch
interfaces.
Here's the program for Marvell CN9130 CRB
https://blackhole.sk/~kabel/src/xdp_mvswitch_route.c
(This is just to experiment with XDP, so please excuse code quality.)

I found out that on Turris MOX I am able to route 2.5gbps (at MTU 1500)
with XDP. But when not using XDP, when the packets go via kernel's
stack, MOX is able to route less than 1gbps (cca 800mbps, at MTU
1500, and the CPU is at 100%).

I also to write a simple NAT masquerading program. I think XDP can
increase NAT throughput to 2.5gbps as well.

> Or i suppose you could modify the tag and send it back to the
> switch? Does XDP support that sort of hairpin operation?

You can modify the packet, prepend it with another headers (up to 256
bytes in some drivers, in mvpp2 only 224), append trailers... Look at
my program above, I am popping vlan tag, for example.

Marek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-05 20:23 ` Andrew Lunn
@ 2021-01-06 12:11   ` Marek Behún
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Behún @ 2021-01-06 12:11 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, davem, Jakub Kicinski, Sven Auhagen, Matteo Croce,
	Lorenzo Bianconi

On Tue, 5 Jan 2021 21:23:12 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> > @@ -4913,8 +4914,8 @@ static int mvpp2_xdp_setup(struct mvpp2_port *port, struct netdev_bpf *bpf)
> >  	bool running = netif_running(port->dev);
> >  	bool reset = !prog != !port->xdp_prog;
> >  
> > -	if (port->dev->mtu > ETH_DATA_LEN) {
> > -		NL_SET_ERR_MSG_MOD(bpf->extack, "XDP is not supported with jumbo frames enabled");
> > +	if (port->dev->mtu > MVPP2_MAX_RX_BUF_SIZE) {
> > +		NL_SET_ERR_MSG_MOD(bpf->extack, "MTU too large for XDP");  
> 
> Hi Marek
> 
> Since MVPP2_MAX_RX_BUF_SIZE is a constant, you can probably do some
> CPP magic and have it part of the extack message, to give the user a
> clue what the maximum is.
> 
>      Andrew

It is a constant that is computed using sizeof of some structs. I don't
know if that can be converted at compile-time to string (it should be,
theoretically, but I don't know if compiler allows it).



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-06 11:56       ` Marek Behún
@ 2021-01-06 12:32         ` Marek Behún
  2021-01-06 12:33           ` Marek Behún
  2021-01-06 18:34           ` Andrew Lunn
  2021-01-06 13:13         ` Vladimir Oltean
  1 sibling, 2 replies; 13+ messages in thread
From: Marek Behún @ 2021-01-06 12:32 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Sven Auhagen, netdev, davem, Jakub Kicinski, Matteo Croce,
	Lorenzo Bianconi

On Wed, 6 Jan 2021 12:56:08 +0100
Marek Behún <kabel@kernel.org> wrote:

> I also to write a simple NAT masquerading program. I think XDP can
> increase NAT throughput to 2.5gbps as well.

BTW currently if XDP modifies the packet, it has to modify the
checksums accordingly. There is a helper for that even, bpf_csum_diff.

But many drivers can offload csum computation, mvneta and mvpp2 for
example. But for this, somehow the XDP program has to let the driver
know what kind of csum it needs to be computed (L3, L4 TCP/UDP).

This could theoretically be communicated with the driver via metadata
prepended to the packet. But a abstraction is needed, so that every
driver does it in the same way. Maybe someone is already working on
this, I don't know...

Marek

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-06 12:32         ` Marek Behún
@ 2021-01-06 12:33           ` Marek Behún
  2021-01-06 18:34           ` Andrew Lunn
  1 sibling, 0 replies; 13+ messages in thread
From: Marek Behún @ 2021-01-06 12:33 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Sven Auhagen, netdev, davem, Jakub Kicinski, Matteo Croce,
	Lorenzo Bianconi

On Wed, 6 Jan 2021 13:32:05 +0100
Marek Behún <kabel@kernel.org> wrote:

> On Wed, 6 Jan 2021 12:56:08 +0100
> Marek Behún <kabel@kernel.org> wrote:
> 
> > I also to write a simple NAT masquerading program. I think XDP can
> > increase NAT throughput to 2.5gbps as well.
> 
> BTW currently if XDP modifies the packet, it has to modify the
> checksums accordingly. There is a helper for that even, bpf_csum_diff.

(from L3 forwards, if you modify ethhdr, you don't have to modify L2
checksum. You can't even see it...)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-06 11:56       ` Marek Behún
  2021-01-06 12:32         ` Marek Behún
@ 2021-01-06 13:13         ` Vladimir Oltean
  1 sibling, 0 replies; 13+ messages in thread
From: Vladimir Oltean @ 2021-01-06 13:13 UTC (permalink / raw)
  To: Marek Behún
  Cc: Andrew Lunn, Sven Auhagen, netdev, davem, Jakub Kicinski,
	Matteo Croce, Lorenzo Bianconi

On Wed, Jan 06, 2021 at 12:56:08PM +0100, Marek Behún wrote:
> I found out that on Turris MOX I am able to route 2.5gbps (at MTU 1500)
> with XDP. But when not using XDP, when the packets go via kernel's
> stack, MOX is able to route less than 1gbps (cca 800mbps, at MTU
> 1500, and the CPU is at 100%).

Read this:
https://patchwork.ozlabs.org/project/netdev/patch/73223229-6bc0-2647-6952-975961811866@gmail.com/

Disable GRO on the DSA interfaces and you'll be just fine even with IP
forwarding done by the network stack. You don't _need_ XDP for this.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled
  2021-01-06 12:32         ` Marek Behún
  2021-01-06 12:33           ` Marek Behún
@ 2021-01-06 18:34           ` Andrew Lunn
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2021-01-06 18:34 UTC (permalink / raw)
  To: Marek Behún
  Cc: Sven Auhagen, netdev, davem, Jakub Kicinski, Matteo Croce,
	Lorenzo Bianconi

On Wed, Jan 06, 2021 at 01:32:05PM +0100, Marek Behún wrote:
> On Wed, 6 Jan 2021 12:56:08 +0100
> Marek Behún <kabel@kernel.org> wrote:
> 
> > I also to write a simple NAT masquerading program. I think XDP can
> > increase NAT throughput to 2.5gbps as well.
> 
> BTW currently if XDP modifies the packet, it has to modify the
> checksums accordingly. There is a helper for that even, bpf_csum_diff.
> 
> But many drivers can offload csum computation, mvneta and mvpp2 for
> example.

Hi Marek

It does require that the MAC is DSA aware. The Freescale FEC for
example cannot do such csum, because the DSA header confuses it, and
it cannot find the IP header, etc. So if you do look at a generic way
to implement this, you need the MAC driver to indicate it has the
needed support.

       Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-01-06 18:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 17:19 [PATCH net-next] net: mvpp2: increase MTU limit when XDP enabled Marek Behún
2021-01-05 17:24 ` Sven Auhagen
2021-01-05 17:43   ` Marek Behún
2021-01-05 20:21     ` Andrew Lunn
2021-01-06 11:56       ` Marek Behún
2021-01-06 12:32         ` Marek Behún
2021-01-06 12:33           ` Marek Behún
2021-01-06 18:34           ` Andrew Lunn
2021-01-06 13:13         ` Vladimir Oltean
2021-01-06 10:33     ` Jesper Dangaard Brouer
2021-01-06 11:28       ` Sven Auhagen
2021-01-05 20:23 ` Andrew Lunn
2021-01-06 12:11   ` Marek Behún

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.