[net PATCH] net: virtio: cap mtu when XDP programs are running

Subsystems: networking drivers, the rest, virtio net driver

STALE3508d

21 messages, 3 authors, 2017-01-11 · open the first message on its own page

[net PATCH] net: virtio: cap mtu when XDP programs are running

From: John Fastabend <john.fastabend@gmail.com>
Date: 2017-01-02 22:30:47 

XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
 drivers/net/virtio_net.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5deeda6..783e842 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1699,6 +1699,9 @@ static void virtnet_init_settings(struct net_device *dev)
 	.set_settings = virtnet_set_settings,
 };
 
+#define MIN_MTU ETH_MIN_MTU
+#define MAX_MTU ETH_MAX_MTU
+
 static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 {
 	unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
@@ -1748,6 +1751,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 			virtnet_set_queues(vi, curr_qp);
 			return PTR_ERR(prog);
 		}
+		dev->max_mtu = max_sz;
+	} else {
+		dev->max_mtu = ETH_MAX_MTU;
 	}
 
 	vi->xdp_queue_pairs = xdp_qp;
@@ -2133,9 +2139,6 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
 	return true;
 }
 
-#define MIN_MTU ETH_MIN_MTU
-#define MAX_MTU ETH_MAX_MTU
-
 static int virtnet_probe(struct virtio_device *vdev)
 {
 	int i, err;

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: Jason Wang <hidden>
Date: 2017-01-03 06:14:55 


On 2017年01月03日 06:30, John Fastabend wrote:
quoted hunk
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
  drivers/net/virtio_net.c |    9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5deeda6..783e842 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1699,6 +1699,9 @@ static void virtnet_init_settings(struct net_device *dev)
  	.set_settings = virtnet_set_settings,
  };
  
+#define MIN_MTU ETH_MIN_MTU
+#define MAX_MTU ETH_MAX_MTU
+
  static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
  {
  	unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
@@ -1748,6 +1751,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
  			virtnet_set_queues(vi, curr_qp);
  			return PTR_ERR(prog);
  		}
+		dev->max_mtu = max_sz;
+	} else {
+		dev->max_mtu = ETH_MAX_MTU;
Or use ETH_DATA_LEN here consider we only allocate a size of 
GOOD_PACKET_LEN for each small buffer?

Thanks
quoted hunk
  	}
  
  	vi->xdp_queue_pairs = xdp_qp;
@@ -2133,9 +2139,6 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
  	return true;
  }
  
-#define MIN_MTU ETH_MIN_MTU
-#define MAX_MTU ETH_MAX_MTU
-
  static int virtnet_probe(struct virtio_device *vdev)
  {
  	int i, err;

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: John Fastabend <john.fastabend@gmail.com>
Date: 2017-01-03 16:48:39 

On 17-01-02 10:14 PM, Jason Wang wrote:

On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
  drivers/net/virtio_net.c |    9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5deeda6..783e842 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1699,6 +1699,9 @@ static void virtnet_init_settings(struct net_device *dev)
      .set_settings = virtnet_set_settings,
  };
  +#define MIN_MTU ETH_MIN_MTU
+#define MAX_MTU ETH_MAX_MTU
+
  static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
  {
      unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
@@ -1748,6 +1751,9 @@ static int virtnet_xdp_set(struct net_device *dev,
struct bpf_prog *prog)
              virtnet_set_queues(vi, curr_qp);
              return PTR_ERR(prog);
          }
+        dev->max_mtu = max_sz;
+    } else {
+        dev->max_mtu = ETH_MAX_MTU;
Or use ETH_DATA_LEN here consider we only allocate a size of GOOD_PACKET_LEN for
each small buffer?

Thanks
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

   virtio_cread16(vdev, ...)

or we may break the negotiated mtu.

As for capping it at GOOD_PACKET_LEN this has the nice benefit of avoiding any
underestimates in EWMA predictions because it appears min estimates are capped
at GOOD_PACKET_LEN via get_mergeable_buf_len().

Thanks,
John

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: John Fastabend <john.fastabend@gmail.com>
Date: 2017-01-03 16:48:59 

On 17-01-02 10:14 PM, Jason Wang wrote:

On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
  drivers/net/virtio_net.c |    9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5deeda6..783e842 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1699,6 +1699,9 @@ static void virtnet_init_settings(struct net_device *dev)
      .set_settings = virtnet_set_settings,
  };
  +#define MIN_MTU ETH_MIN_MTU
+#define MAX_MTU ETH_MAX_MTU
+
  static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
  {
      unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
@@ -1748,6 +1751,9 @@ static int virtnet_xdp_set(struct net_device *dev,
struct bpf_prog *prog)
              virtnet_set_queues(vi, curr_qp);
              return PTR_ERR(prog);
          }
+        dev->max_mtu = max_sz;
+    } else {
+        dev->max_mtu = ETH_MAX_MTU;
Or use ETH_DATA_LEN here consider we only allocate a size of GOOD_PACKET_LEN for
each small buffer?

Thanks
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

   virtio_cread16(vdev, ...)

or we may break the negotiated mtu.

As for capping it at GOOD_PACKET_LEN this has the nice benefit of avoiding any
underestimates in EWMA predictions because it appears min estimates are capped
at GOOD_PACKET_LEN via get_mergeable_buf_len().

Thanks,
John

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: Jason Wang <hidden>
Date: 2017-01-04 03:16:23 

  case.



On 2017年01月04日 00:48, John Fastabend wrote:
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
   drivers/net/virtio_net.c |    9 ++++++---
   1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5deeda6..783e842 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1699,6 +1699,9 @@ static void virtnet_init_settings(struct net_device *dev)
       .set_settings = virtnet_set_settings,
   };
   +#define MIN_MTU ETH_MIN_MTU
+#define MAX_MTU ETH_MAX_MTU
+
   static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
   {
       unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
@@ -1748,6 +1751,9 @@ static int virtnet_xdp_set(struct net_device *dev,
struct bpf_prog *prog)
               virtnet_set_queues(vi, curr_qp);
               return PTR_ERR(prog);
           }
+        dev->max_mtu = max_sz;
+    } else {
+        dev->max_mtu = ETH_MAX_MTU;
Or use ETH_DATA_LEN here consider we only allocate a size of GOOD_PACKET_LEN for
each small buffer?

Thanks
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

    virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to 
notify the device about the mtu in this case which is not supported by 
virtio now.
As for capping it at GOOD_PACKET_LEN this has the nice benefit of avoiding any
underestimates in EWMA predictions because it appears min estimates are capped
at GOOD_PACKET_LEN via get_mergeable_buf_len().
This seems something misunderstanding here, I meant only use 
GOOD_PACKET_LEN for small buffer (which does not use EWMA).

Thanks

Thanks,
John

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: John Fastabend <john.fastabend@gmail.com>
Date: 2017-01-04 18:58:04 

[...]

On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
[...]
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

    virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
quoted
As for capping it at GOOD_PACKET_LEN this has the nice benefit of avoiding any
underestimates in EWMA predictions because it appears min estimates are capped
at GOOD_PACKET_LEN via get_mergeable_buf_len().
This seems something misunderstanding here, I meant only use GOOD_PACKET_LEN for
small buffer (which does not use EWMA).
Yep I think its all cleared up now.

Thanks.

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: Jason Wang <hidden>
Date: 2017-01-05 03:09:22 


On 2017年01月05日 02:57, John Fastabend wrote:
[...]
quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
[...]
quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

     virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host 
about MTU change in this case?

Thanks

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2017-01-09 23:05:59 

On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:

On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]
quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
[...]
quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

     virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: John Fastabend <john.fastabend@gmail.com>
Date: 2017-01-09 23:13:37 

On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted

On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]
quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
[...]
quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

     virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced. Although the guest
by reducing its MTU in some sense must expect this. Likewise if the host were
to change MTU after virtio_net probe time the guest would not learn about it.

I think at best negotiating the mtu is just a hint? If system _really_ cares
we could use lldp or some other out of band mechanism to learn/set/adjust MTU
on both systems and it would be more robust. I'm not actually convinced this
is a problem in bare metal systems we have the same issue with physical
switches and solve it out of band via configuration, protocols, etc.

.John

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2017-01-09 23:24:47 

On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted

On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]
quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
[...]
quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

     virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?

Although the guest
by reducing its MTU in some sense must expect this. Likewise if the host were
to change MTU after virtio_net probe time the guest would not learn about it.
The spec explicitly disallows this last one.

I think at best negotiating the mtu is just a hint? If system _really_ cares
we could use lldp or some other out of band mechanism to learn/set/adjust MTU
on both systems and it would be more robust. I'm not actually convinced this
is a problem in bare metal systems we have the same issue with physical
switches and solve it out of band via configuration, protocols, etc.

.John
ATM we don't have negotiation in virtio, just a max mtu limit.
This doesn't free guest from configuring mtu correctly,
just helps it avoid doing something clearly bogus.

-- 
MST

Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

From: John Fastabend <john.fastabend@gmail.com>
Date: 2017-01-09 23:49:50 

On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted

On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]
quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
[...]
quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

     virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
quoted
Although the guest
by reducing its MTU in some sense must expect this. Likewise if the host were
to change MTU after virtio_net probe time the guest would not learn about it.
The spec explicitly disallows this last one.
OK. By the way were do I get the latest source I see the published virtio1.0 at
the oasis-open.org site but it doesn't mention the MTU logic.

quoted
I think at best negotiating the mtu is just a hint? If system _really_ cares
we could use lldp or some other out of band mechanism to learn/set/adjust MTU
on both systems and it would be more robust. I'm not actually convinced this
is a problem in bare metal systems we have the same issue with physical
switches and solve it out of band via configuration, protocols, etc.

.John
ATM we don't have negotiation in virtio, just a max mtu limit.
This doesn't free guest from configuring mtu correctly,
just helps it avoid doing something clearly bogus.
Yep. I'm fine with calling it a misconfiguration if the guest reduces the MTU
and the host continues to send packets @ advertised MTU.





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: "Michael S. Tsirkin" <mst@redhat.com>

      Date: 2017-01-09 23:59:00 
    

  

  
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted

On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend <redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

     virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.


quoted
quoted
Although the guest
by reducing its MTU in some sense must expect this. Likewise if the host were
to change MTU after virtio_net probe time the guest would not learn about it.
The spec explicitly disallows this last one.
OK. By the way were do I get the latest source I see the published virtio1.0 at
the oasis-open.org site but it doesn't mention the MTU logic.
You need to get it from svn.

quoted
quoted
I think at best negotiating the mtu is just a hint? If system _really_ cares
we could use lldp or some other out of band mechanism to learn/set/adjust MTU
on both systems and it would be more robust. I'm not actually convinced this
is a problem in bare metal systems we have the same issue with physical
switches and solve it out of band via configuration, protocols, etc.

.John
ATM we don't have negotiation in virtio, just a max mtu limit.
This doesn't free guest from configuring mtu correctly,
just helps it avoid doing something clearly bogus.
Yep. I'm fine with calling it a misconfiguration if the guest reduces the MTU
and the host continues to send packets @ advertised MTU.





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: Jason Wang <hidden>

      Date: 2017-01-10 02:29:48 
    

  

  

On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

      virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with 
size of GOOD_PACKET_LEN? I think in any case, we should limit max_mtu to 
GOOD_PACKET_LEN for small buffers.

Thanks





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: "Michael S. Tsirkin" <mst@redhat.com>

      Date: 2017-01-10 02:51:36 
    

  

  
On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:

On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

      virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with size
of GOOD_PACKET_LEN?
Spec says hardware won't send in packets > max mtu in config space.

I think in any case, we should limit max_mtu to
GOOD_PACKET_LEN for small buffers.

Thanks
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?

-- 
MST





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: John Fastabend <john.fastabend@gmail.com>

      Date: 2017-01-10 03:31:03 
    

  

  
On 17-01-09 06:51 PM, Michael S. Tsirkin wrote:
On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:
quoted

On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

      virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with size
of GOOD_PACKET_LEN?
Spec says hardware won't send in packets > max mtu in config space.

quoted
I think in any case, we should limit max_mtu to
GOOD_PACKET_LEN for small buffers.

Thanks
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?
OK I agree I'll put most the logic in rtnetlink.c when the program is added
or removed.

But, I'm looking at the non-XDP receive_small path now and wondering how does
multiple buffer receives work (e.g. packet larger than GOOD_PACKET_LEN?) I think
this is what Jason is looking at as well? The mergeable case clearly looks at
num_bufs in the descriptor to construct multi-buffer packets but nothing like
that exists in the small_receive path as best I can tell.

.John





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: Jason Wang <hidden>

      Date: 2017-01-10 03:34:32 
    

  

  

On 2017年01月10日 10:51, Michael S. Tsirkin wrote:
On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:
quoted
On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

       virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with size
of GOOD_PACKET_LEN?
Spec says hardware won't send in packets > max mtu in config space.
Yes, but if max mtu is greater than GOOD_PACKET_LEN, packet will be dropped.

quoted
I think in any case, we should limit max_mtu to
GOOD_PACKET_LEN for small buffers.

Thanks
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?
Not sure it's a good idea to change mtu silently without notifying user.

Thanks





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: "Michael S. Tsirkin" <mst@redhat.com>

      Date: 2017-01-10 03:55:41 
    

  

  
On Mon, Jan 09, 2017 at 07:30:34PM -0800, John Fastabend wrote:
On 17-01-09 06:51 PM, Michael S. Tsirkin wrote:
quoted
On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:
quoted

On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

      virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with size
of GOOD_PACKET_LEN?
Spec says hardware won't send in packets > max mtu in config space.

quoted
I think in any case, we should limit max_mtu to
GOOD_PACKET_LEN for small buffers.

Thanks
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?
OK I agree I'll put most the logic in rtnetlink.c when the program is added
or removed.

But, I'm looking at the non-XDP receive_small path now and wondering how does
multiple buffer receives work (e.g. packet larger than GOOD_PACKET_LEN?)
I don't understand the question. Look at add_recvbuf_small,
it adds a tiny buffer for head and then the skb.


I think
this is what Jason is looking at as well? The mergeable case clearly looks at
num_bufs in the descriptor to construct multi-buffer packets but nothing like
that exists in the small_receive path as best I can tell.

.John
There's always a single buffer there.
BTW it was always a legacy path but if it's now important for people we
should probably check ANY_LAYOUT and put header linearly with the packet
if there.

-- 
MST





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: John Fastabend <john.fastabend@gmail.com>

      Date: 2017-01-10 04:25:59 
    

  

  
On 17-01-09 07:55 PM, Michael S. Tsirkin wrote:
On Mon, Jan 09, 2017 at 07:30:34PM -0800, John Fastabend wrote:
quoted
On 17-01-09 06:51 PM, Michael S. Tsirkin wrote:
quoted
On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:
quoted

On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

      virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with size
of GOOD_PACKET_LEN?
Spec says hardware won't send in packets > max mtu in config space.

quoted
I think in any case, we should limit max_mtu to
GOOD_PACKET_LEN for small buffers.

Thanks
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?
OK I agree I'll put most the logic in rtnetlink.c when the program is added
or removed.

But, I'm looking at the non-XDP receive_small path now and wondering how does
multiple buffer receives work (e.g. packet larger than GOOD_PACKET_LEN?)
I don't understand the question. Look at add_recvbuf_small,
it adds a tiny buffer for head and then the skb.
Specifically this seems to fail with mergeable buffers disabled

On the host:

# ip link set dev tap0 mtu 9000
# ping 22.2 -s 2048

On the guest:

# insmod ./drivers/net/virtio_net.ko
# ip link set dev eth0 mtu 9000

With mergeable buffers enabled no problems it works as I expect at least.


quoted
I think
this is what Jason is looking at as well? The mergeable case clearly looks at
num_bufs in the descriptor to construct multi-buffer packets but nothing like
that exists in the small_receive path as best I can tell.

.John
There's always a single buffer there.
BTW it was always a legacy path but if it's now important for people we
should probably check ANY_LAYOUT and put header linearly with the packet
if there.





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: "Michael S. Tsirkin" <mst@redhat.com>

      Date: 2017-01-10 05:00:59 
    

  

  
On Mon, Jan 09, 2017 at 08:25:43PM -0800, John Fastabend wrote:
On 17-01-09 07:55 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 07:30:34PM -0800, John Fastabend wrote:
quoted
On 17-01-09 06:51 PM, Michael S. Tsirkin wrote:
quoted
On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:
quoted

On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

      virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with size
of GOOD_PACKET_LEN?
Spec says hardware won't send in packets > max mtu in config space.

quoted
I think in any case, we should limit max_mtu to
GOOD_PACKET_LEN for small buffers.

Thanks
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?
OK I agree I'll put most the logic in rtnetlink.c when the program is added
or removed.

But, I'm looking at the non-XDP receive_small path now and wondering how does
multiple buffer receives work (e.g. packet larger than GOOD_PACKET_LEN?)
I don't understand the question. Look at add_recvbuf_small,
it adds a tiny buffer for head and then the skb.
Specifically this seems to fail with mergeable buffers disabled

On the host:

# ip link set dev tap0 mtu 9000
# ping 22.2 -s 2048

On the guest:

# insmod ./drivers/net/virtio_net.ko
# ip link set dev eth0 mtu 9000
Why would it work? You are sending a packet larger than ethernet MTU.

With mergeable buffers enabled no problems it works as I expect at least.
We don't expect to get these packets but
mergeable is able to process them anyway.
It's an accident :) 

quoted
quoted
I think
this is what Jason is looking at as well? The mergeable case clearly looks at
num_bufs in the descriptor to construct multi-buffer packets but nothing like
that exists in the small_receive path as best I can tell.

.John
There's always a single buffer there.
BTW it was always a legacy path but if it's now important for people we
should probably check ANY_LAYOUT and put header linearly with the packet
if there.





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: "Michael S. Tsirkin" <mst@redhat.com>

      Date: 2017-01-10 14:51:06 
    

  

  
On Tue, Jan 10, 2017 at 04:51:34AM +0200, Michael S. Tsirkin wrote:
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?
And BTW limits on MTU are a problem that will have to be
addressed sooner or later, disabling offloads on the NIC
is one thing, but reconfiguring all of the network
with a lower MTU is another.


-- 
MST





  

    
Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

    

      From: Jason Wang <hidden>

      Date: 2017-01-11 03:37:17 
    

  

  

On 2017年01月10日 13:00, Michael S. Tsirkin wrote:
On Mon, Jan 09, 2017 at 08:25:43PM -0800, John Fastabend wrote:
quoted
On 17-01-09 07:55 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 07:30:34PM -0800, John Fastabend wrote:
quoted
On 17-01-09 06:51 PM, Michael S. Tsirkin wrote:
quoted
On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:
quoted
On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
quoted
On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
quoted
On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
quoted
On 2017年01月05日 02:57, John Fastabend wrote:
quoted
[...]

quoted
On 2017年01月04日 00:48, John Fastabend wrote:
quoted
On 17-01-02 10:14 PM, Jason Wang wrote:
quoted
On 2017年01月03日 06:30, John Fastabend wrote:
quoted
XDP programs can not consume multiple pages so we cap the MTU to
avoid this case. Virtio-net however only checks the MTU at XDP
program load and does not block MTU changes after the program
has loaded.

This patch sets/clears the max_mtu value at XDP load/unload time.

Signed-off-by: John Fastabend<redacted>
---
[...]

quoted
quoted
OK so this logic is a bit too simply. When it resets the max_mtu I guess it
needs to read the mtu via

       virtio_cread16(vdev, ...)

or we may break the negotiated mtu.
Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
the device about the mtu in this case which is not supported by virtio now.
Note this is not really a XDP specific problem. The guest can change the MTU
after init time even without XDP which I assume should ideally result in a
notification if the MTU is negotiated.
Yes, Michael, do you think we need add some mechanism to notify host about
MTU change in this case?

Thanks
Why does host care?
Well the guest will drop packets after mtu has been reduced.
I didn't know. What place in code does this?
hmm in many of the drivers it is convention to use the mtu to set the rx
buffer sizes and a receive side max length filter. For example in the Intel
drivers if a packet with length greater than MTU + some headroom is received we
drop it. I guess in the networking stack RX path though nothing forces this and
virtio doesn't have any code to drop packets on rx size.

In virtio I don't see any existing case currently. In the XDP case though we
need to ensure packets fit in a page for the time being which is why I was
looking at this code and generated this patch.
I'd say just look at the hardware max mtu. Ignore the configured mtu.

Does this work for small buffers consider it always allocate skb with size
of GOOD_PACKET_LEN?
Spec says hardware won't send in packets > max mtu in config space.

quoted
I think in any case, we should limit max_mtu to
GOOD_PACKET_LEN for small buffers.

Thanks
XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?
OK I agree I'll put most the logic in rtnetlink.c when the program is added
or removed.

But, I'm looking at the non-XDP receive_small path now and wondering how does
multiple buffer receives work (e.g. packet larger than GOOD_PACKET_LEN?)
I don't understand the question. Look at add_recvbuf_small,
it adds a tiny buffer for head and then the skb.
Specifically this seems to fail with mergeable buffers disabled

On the host:

# ip link set dev tap0 mtu 9000
# ping 22.2 -s 2048

On the guest:

# insmod ./drivers/net/virtio_net.ko
# ip link set dev eth0 mtu 9000
Why would it work? You are sending a packet larger than ethernet MTU.
Ok, does it mean virtio-net does not support Jumbo frame? And if it 
can't work, use MAX_MTU as max_mtu is a bug to me.

quoted
With mergeable buffers enabled no problems it works as I expect at least.
We don't expect to get these packets but
mergeable is able to process them anyway.
It's an accident:)  
But path MTU discovery indeed benefits from this "accident".

Thanks




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help