On Mon, Jan 09, 2017 at 07:30:34PM -0800, John Fastabend wrote: > On 17-01-09 06:51 PM, Michael S. Tsirkin wrote: > > On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote: > >> > >> > >> On 2017年01月10日 07:58, Michael S. Tsirkin wrote: > >>> On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote: > >>>> 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.fastab...@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. > >>> 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? > > > > 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
