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

[John Fastabend <john.fastabend@gmail.com>]

On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
>> On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
>>> On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年01月05日 02:57, John Fastabend wrote:
>>>>> [...]
>>>>>
>>>>>> On 2017年01月04日 00:48, John Fastabend wrote:
>>>>>>> On 17-01-02 10:14 PM, Jason Wang wrote:
>>>>>>>> On 2017年01月03日 06:30, John Fastabend wrote:
>>>>>>>>> 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 <john.r.fastabend@intel.com>
>>>>>>>>> ---
>>>>> [...]
>>>>>
>>>>>>> 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.

>> 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.

> 
>> 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.