Jason Wang
2020-Feb-11 02:51 UTC
[PATCH] virtio: Work around frames incorrectly marked as gso
On 2020/2/11 ??12:55, Anton Ivanov wrote:> > > On 09/12/2019 10:48, anton.ivanov at cambridgegreys.com wrote: >> From: Anton Ivanov <anton.ivanov at cambridgegreys.com> >> >> Some of the frames marked as GSO which arrive at >> virtio_net_hdr_from_skb() have no GSO_TYPE, no >> fragments (data_len = 0) and length significantly shorter >> than the MTU (752 in my experiments). >> >> This is observed on raw sockets reading off vEth interfaces >> in all 4.x and 5.x kernels I tested. >> >> These frames are reported as invalid while they are in fact >> gso-less frames. >> >> This patch marks the vnet header as no-GSO for them instead >> of reporting it as invalid. >> >> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com> >> --- >> ? include/linux/virtio_net.h | 8 ++++++-- >> ? 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >> index 0d1fe9297ac6..d90d5cff1b9a 100644 >> --- a/include/linux/virtio_net.h >> +++ b/include/linux/virtio_net.h >> @@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const >> struct sk_buff *skb, >> ????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4; >> ????????? else if (sinfo->gso_type & SKB_GSO_TCPV6) >> ????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6; >> -??????? else >> -??????????? return -EINVAL; >> +??????? else { >> +??????????? if (skb->data_len == 0) >> +??????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE; >> +??????????? else >> +??????????????? return -EINVAL; >> +??????? } >> ????????? if (sinfo->gso_type & SKB_GSO_TCP_ECN) >> ????????????? hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN; >> ????? } else >> > > ping. >Do you mean gso_size is set but gso_type is not? Looks like a bug elsewhere. Thanks
Anton Ivanov
2020-Feb-11 07:42 UTC
[PATCH] virtio: Work around frames incorrectly marked as gso
On 11/02/2020 02:51, Jason Wang wrote:> > On 2020/2/11 ??12:55, Anton Ivanov wrote: >> >> >> On 09/12/2019 10:48, anton.ivanov at cambridgegreys.com wrote: >>> From: Anton Ivanov <anton.ivanov at cambridgegreys.com> >>> >>> Some of the frames marked as GSO which arrive at >>> virtio_net_hdr_from_skb() have no GSO_TYPE, no >>> fragments (data_len = 0) and length significantly shorter >>> than the MTU (752 in my experiments). >>> >>> This is observed on raw sockets reading off vEth interfaces >>> in all 4.x and 5.x kernels I tested. >>> >>> These frames are reported as invalid while they are in fact >>> gso-less frames. >>> >>> This patch marks the vnet header as no-GSO for them instead >>> of reporting it as invalid. >>> >>> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com> >>> --- >>> ? include/linux/virtio_net.h | 8 ++++++-- >>> ? 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h >>> index 0d1fe9297ac6..d90d5cff1b9a 100644 >>> --- a/include/linux/virtio_net.h >>> +++ b/include/linux/virtio_net.h >>> @@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const >>> struct sk_buff *skb, >>> ????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4; >>> ????????? else if (sinfo->gso_type & SKB_GSO_TCPV6) >>> ????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6; >>> -??????? else >>> -??????????? return -EINVAL; >>> +??????? else { >>> +??????????? if (skb->data_len == 0) >>> +??????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE; >>> +??????????? else >>> +??????????????? return -EINVAL; >>> +??????? } >>> ????????? if (sinfo->gso_type & SKB_GSO_TCP_ECN) >>> ????????????? hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN; >>> ????? } else >>> >> >> ping. >> > > Do you mean gso_size is set but gso_type is not? Looks like a bug > elsewhere. > > Thanks > >Yes. I could not trace it where it is coming from. I see it when doing recvmmsg on raw sockets in the UML vector network drivers. -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/
Michael S. Tsirkin
2020-Feb-11 10:37 UTC
[PATCH] virtio: Work around frames incorrectly marked as gso
On Tue, Feb 11, 2020 at 07:42:37AM +0000, Anton Ivanov wrote:> On 11/02/2020 02:51, Jason Wang wrote: > > > > On 2020/2/11 ??12:55, Anton Ivanov wrote: > > > > > > > > > On 09/12/2019 10:48, anton.ivanov at cambridgegreys.com wrote: > > > > From: Anton Ivanov <anton.ivanov at cambridgegreys.com> > > > > > > > > Some of the frames marked as GSO which arrive at > > > > virtio_net_hdr_from_skb() have no GSO_TYPE, no > > > > fragments (data_len = 0) and length significantly shorter > > > > than the MTU (752 in my experiments). > > > > > > > > This is observed on raw sockets reading off vEth interfaces > > > > in all 4.x and 5.x kernels I tested. > > > > > > > > These frames are reported as invalid while they are in fact > > > > gso-less frames. > > > > > > > > This patch marks the vnet header as no-GSO for them instead > > > > of reporting it as invalid. > > > > > > > > Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com> > > > > --- > > > > ? include/linux/virtio_net.h | 8 ++++++-- > > > > ? 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > > > > index 0d1fe9297ac6..d90d5cff1b9a 100644 > > > > --- a/include/linux/virtio_net.h > > > > +++ b/include/linux/virtio_net.h > > > > @@ -112,8 +112,12 @@ static inline int > > > > virtio_net_hdr_from_skb(const struct sk_buff *skb, > > > > ????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4; > > > > ????????? else if (sinfo->gso_type & SKB_GSO_TCPV6) > > > > ????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6; > > > > -??????? else > > > > -??????????? return -EINVAL; > > > > +??????? else { > > > > +??????????? if (skb->data_len == 0) > > > > +??????????????? hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE; > > > > +??????????? else > > > > +??????????????? return -EINVAL; > > > > +??????? } > > > > ????????? if (sinfo->gso_type & SKB_GSO_TCP_ECN) > > > > ????????????? hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN; > > > > ????? } else > > > > > > > > > > ping. > > > > > > > Do you mean gso_size is set but gso_type is not? Looks like a bug > > elsewhere. > > > > Thanks > > > > > Yes. > > I could not trace it where it is coming from. > > I see it when doing recvmmsg on raw sockets in the UML vector network > drivers. >I think we need to find the culprit and fix it there, lots of other things can break otherwise. Just printing out skb->dev->name should do the trick, no?> -- > Anton R. Ivanov > Cambridgegreys Limited. Registered in England. Company Number 10273661 > https://www.cambridgegreys.com/
Reasonably Related Threads
- [PATCH] virtio: Work around frames incorrectly marked as gso
- [PATCH] virtio: Work around frames incorrectly marked as gso
- [PATCH] virtio: Work around frames incorrectly marked as gso
- [PATCH] virtio: Work around frames incorrectly marked as gso
- [PATCH] virtio: Work around frames incorrectly marked as gso