* [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures [not found] <cover.1792067836.git.mst@redhat.com> @ 2026-10-15 12:41 ` Michael S. Tsirkin 2026-10-15 12:47 ` Johannes Berg 2026-10-15 17:19 ` Willem de Bruijn 2026-10-15 12:41 ` [PATCH net-next v2 3/7] um: vector: trim skbs before verifying the header Michael S. Tsirkin 1 sibling, 2 replies; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-15 12:41 UTC (permalink / raw) To: linux-kernel Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um virtio_net_hdr_to_skb() refuses packets whose checksum metadata it cannot make sense of, but the vector raw transport ignored its return value: rejected packets went on into the stack with their metadata unvalidated. Propagate the failure. Assisted-by: LLM Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- arch/um/drivers/vector_transports.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c index e1fb2a76fdff..8b7548458663 100644 --- a/arch/um/drivers/vector_transports.c +++ b/arch/um/drivers/vector_transports.c @@ -210,7 +210,9 @@ static int raw_verify_header( return 1; skb_set_network_header(skb, ETH_HLEN); - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); + if (virtio_net_hdr_to_skb(skb, vheader, + virtio_legacy_is_little_endian()) < 0) + return -1; return 0; } -- MST ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-15 12:41 ` [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures Michael S. Tsirkin @ 2026-10-15 12:47 ` Johannes Berg 2026-10-15 12:52 ` Michael S. Tsirkin 2026-10-15 17:19 ` Willem de Bruijn 1 sibling, 1 reply; 13+ messages in thread From: Johannes Berg @ 2026-10-15 12:47 UTC (permalink / raw) To: Michael S. Tsirkin, linux-kernel Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote: > virtio_net_hdr_to_skb() refuses packets whose checksum metadata > it cannot make sense of, but the vector raw transport ignored > its return value: rejected packets went on into the stack with > their metadata unvalidated. > > Propagate the failure. > > Assisted-by: LLM > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > arch/um/drivers/vector_transports.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > index e1fb2a76fdff..8b7548458663 100644 > --- a/arch/um/drivers/vector_transports.c > +++ b/arch/um/drivers/vector_transports.c > @@ -210,7 +210,9 @@ static int raw_verify_header( > return 1; > > skb_set_network_header(skb, ETH_HLEN); > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > + if (virtio_net_hdr_to_skb(skb, vheader, > + virtio_legacy_is_little_endian()) < 0) > + return -1; > return 0; > Seems it'd be simpler to just return virtio_net_hdr_to_skb(...); As far as I can tell, the function only returns -EINVAL or 0 (never a positive value), and the caller only cares about negative (error) or positive (no checksum needed), so that'd be equivalent? johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-15 12:47 ` Johannes Berg @ 2026-10-15 12:52 ` Michael S. Tsirkin 2026-10-15 12:55 ` Johannes Berg 0 siblings, 1 reply; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-15 12:52 UTC (permalink / raw) To: Johannes Berg Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Thu, Oct 15, 2026 at 02:47:01PM +0200, Johannes Berg wrote: > On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote: > > virtio_net_hdr_to_skb() refuses packets whose checksum metadata > > it cannot make sense of, but the vector raw transport ignored > > its return value: rejected packets went on into the stack with > > their metadata unvalidated. > > > > Propagate the failure. > > > > Assisted-by: LLM > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > arch/um/drivers/vector_transports.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > > index e1fb2a76fdff..8b7548458663 100644 > > --- a/arch/um/drivers/vector_transports.c > > +++ b/arch/um/drivers/vector_transports.c > > @@ -210,7 +210,9 @@ static int raw_verify_header( > > return 1; > > > > skb_set_network_header(skb, ETH_HLEN); > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > + if (virtio_net_hdr_to_skb(skb, vheader, > > + virtio_legacy_is_little_endian()) < 0) > > + return -1; > > return 0; > > > > Seems it'd be simpler to just > > return virtio_net_hdr_to_skb(...); > > As far as I can tell, the function only returns -EINVAL or 0 (never a > positive value), and the caller only cares about negative (error) or > positive (no checksum needed), so that'd be equivalent? > > johannes if raw_verify_header can safely return any errno - i did not want to audit it. -- MST ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-15 12:52 ` Michael S. Tsirkin @ 2026-10-15 12:55 ` Johannes Berg 2026-10-16 9:37 ` Michael S. Tsirkin 2026-10-16 9:43 ` Michael S. Tsirkin 0 siblings, 2 replies; 13+ messages in thread From: Johannes Berg @ 2026-10-15 12:55 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Thu, 2026-10-15 at 08:52 -0400, Michael S. Tsirkin wrote: > On Thu, Oct 15, 2026 at 02:47:01PM +0200, Johannes Berg wrote: > > On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote: > > > virtio_net_hdr_to_skb() refuses packets whose checksum metadata > > > it cannot make sense of, but the vector raw transport ignored > > > its return value: rejected packets went on into the stack with > > > their metadata unvalidated. > > > > > > Propagate the failure. > > > > > > Assisted-by: LLM > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > > arch/um/drivers/vector_transports.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > > > index e1fb2a76fdff..8b7548458663 100644 > > > --- a/arch/um/drivers/vector_transports.c > > > +++ b/arch/um/drivers/vector_transports.c > > > @@ -210,7 +210,9 @@ static int raw_verify_header( > > > return 1; > > > > > > skb_set_network_header(skb, ETH_HLEN); > > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > > + if (virtio_net_hdr_to_skb(skb, vheader, > > > + virtio_legacy_is_little_endian()) < 0) > > > + return -1; > > > return 0; > > > > > > > Seems it'd be simpler to just > > > > return virtio_net_hdr_to_skb(...); > > > > As far as I can tell, the function only returns -EINVAL or 0 (never a > > positive value), and the caller only cares about negative (error) or > > positive (no checksum needed), so that'd be equivalent? > > > > johannes > > if raw_verify_header can safely return any errno - i did not want to > audit it. You already used an LLM and still didn't want to? ;-) I checked briefly (by hand) and it seems fine, it's assigned to the function pointer, which only has two callers on the _kern.c side. johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-15 12:55 ` Johannes Berg @ 2026-10-16 9:37 ` Michael S. Tsirkin 2026-10-16 9:43 ` Michael S. Tsirkin 1 sibling, 0 replies; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-16 9:37 UTC (permalink / raw) To: Johannes Berg Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Thu, Oct 15, 2026 at 02:55:23PM +0200, Johannes Berg wrote: > On Thu, 2026-10-15 at 08:52 -0400, Michael S. Tsirkin wrote: > > On Thu, Oct 15, 2026 at 02:47:01PM +0200, Johannes Berg wrote: > > > On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote: > > > > virtio_net_hdr_to_skb() refuses packets whose checksum metadata > > > > it cannot make sense of, but the vector raw transport ignored > > > > its return value: rejected packets went on into the stack with > > > > their metadata unvalidated. > > > > > > > > Propagate the failure. > > > > > > > > Assisted-by: LLM > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > --- > > > > arch/um/drivers/vector_transports.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > > > > index e1fb2a76fdff..8b7548458663 100644 > > > > --- a/arch/um/drivers/vector_transports.c > > > > +++ b/arch/um/drivers/vector_transports.c > > > > @@ -210,7 +210,9 @@ static int raw_verify_header( > > > > return 1; > > > > > > > > skb_set_network_header(skb, ETH_HLEN); > > > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > > > + if (virtio_net_hdr_to_skb(skb, vheader, > > > > + virtio_legacy_is_little_endian()) < 0) > > > > + return -1; > > > > return 0; > > > > > > > > > > Seems it'd be simpler to just > > > > > > return virtio_net_hdr_to_skb(...); > > > > > > As far as I can tell, the function only returns -EINVAL or 0 (never a > > > positive value), and the caller only cares about negative (error) or > > > positive (no checksum needed), so that'd be equivalent? > > > > > > johannes > > > > if raw_verify_header can safely return any errno - i did not want to > > audit it. > > You already used an LLM and still didn't want to? ;-) I don't really trust it to do code audits especially in code I do not test. > I checked briefly (by hand) and it seems fine, it's assigned to the > function pointer, which only has two callers on the _kern.c side. > > johannes Thanks, will make the change. -- MST ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-15 12:55 ` Johannes Berg 2026-10-16 9:37 ` Michael S. Tsirkin @ 2026-10-16 9:43 ` Michael S. Tsirkin 2026-10-16 9:44 ` Johannes Berg 1 sibling, 1 reply; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-16 9:43 UTC (permalink / raw) To: Johannes Berg Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Thu, Oct 15, 2026 at 02:55:23PM +0200, Johannes Berg wrote: > On Thu, 2026-10-15 at 08:52 -0400, Michael S. Tsirkin wrote: > > On Thu, Oct 15, 2026 at 02:47:01PM +0200, Johannes Berg wrote: > > > On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote: > > > > virtio_net_hdr_to_skb() refuses packets whose checksum metadata > > > > it cannot make sense of, but the vector raw transport ignored > > > > its return value: rejected packets went on into the stack with > > > > their metadata unvalidated. > > > > > > > > Propagate the failure. > > > > > > > > Assisted-by: LLM > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > --- > > > > arch/um/drivers/vector_transports.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > > > > index e1fb2a76fdff..8b7548458663 100644 > > > > --- a/arch/um/drivers/vector_transports.c > > > > +++ b/arch/um/drivers/vector_transports.c > > > > @@ -210,7 +210,9 @@ static int raw_verify_header( > > > > return 1; > > > > > > > > skb_set_network_header(skb, ETH_HLEN); > > > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > > > + if (virtio_net_hdr_to_skb(skb, vheader, > > > > + virtio_legacy_is_little_endian()) < 0) > > > > + return -1; > > > > return 0; > > > > > > > > > > Seems it'd be simpler to just > > > > > > return virtio_net_hdr_to_skb(...); > > > > > > As far as I can tell, the function only returns -EINVAL or 0 (never a > > > positive value), and the caller only cares about negative (error) or > > > positive (no checksum needed), so that'd be equivalent? > > > > > > johannes > > > > if raw_verify_header can safely return any errno - i did not want to > > audit it. > > You already used an LLM and still didn't want to? ;-) > > I checked briefly (by hand) and it seems fine, it's assigned to the > function pointer, which only has two callers on the _kern.c side. > > johannes header_check = vp->verify_header( mmsg_vector->msg_hdr.msg_iov->iov_base, skb, vp ); if (header_check < 0) { /* Overlay header failed to verify - discard. * We can actually keep this skb and reuse it, * but that will make the prep logic too * complex. */ dev_kfree_skb_irq(skb); vp->estats.rx_encaps_errors++; (*skbuff_vector) = NULL; mmsg_vector++; skbuff_vector++; continue; } if (header_check > 0) { vp->estats.rx_csum_offload_good++; skb->ip_summed = CHECKSUM_UNNECESSARY; } so any negative is fine, but if we return 1 then it will force to CHECKSUM_UNNECESSARY which I am not sure is what was intended? -- MST ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-16 9:43 ` Michael S. Tsirkin @ 2026-10-16 9:44 ` Johannes Berg 2026-10-16 9:47 ` Michael S. Tsirkin 0 siblings, 1 reply; 13+ messages in thread From: Johannes Berg @ 2026-10-16 9:44 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Fri, 2026-10-16 at 05:43 -0400, Michael S. Tsirkin wrote: > > > > > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > > > > + if (virtio_net_hdr_to_skb(skb, vheader, > > > > > + virtio_legacy_is_little_endian()) < 0) > > > > > + return -1; > > > > > return 0; > > > > > > > > > > > > > Seems it'd be simpler to just > > > > > > > > return virtio_net_hdr_to_skb(...); > > > > > > > > As far as I can tell, the function only returns -EINVAL or 0 (never a > > > > positive value), and the caller only cares about negative (error) or > > > > positive (no checksum needed), so that'd be equivalent? > > > > > > > > johannes > > > > > > if raw_verify_header can safely return any errno - i did not want to > > > audit it. > > > > You already used an LLM and still didn't want to? ;-) > > > > I checked briefly (by hand) and it seems fine, it's assigned to the > > function pointer, which only has two callers on the _kern.c side. > > > > johannes > > > header_check = vp->verify_header( > mmsg_vector->msg_hdr.msg_iov->iov_base, > skb, > vp > ); > if (header_check < 0) { > /* Overlay header failed to verify - discard. > * We can actually keep this skb and reuse it, > * but that will make the prep logic too > * complex. > */ > dev_kfree_skb_irq(skb); > vp->estats.rx_encaps_errors++; > (*skbuff_vector) = NULL; > mmsg_vector++; > skbuff_vector++; > continue; > } > if (header_check > 0) { > vp->estats.rx_csum_offload_good++; > skb->ip_summed = CHECKSUM_UNNECESSARY; > } > > > > so any negative is fine, but if we return 1 then it will force to CHECKSUM_UNNECESSARY > which I am not sure is what was intended? > Right, but virtio_net_hdr_to_skb() cannot return 1, only 0 on success and -ERROR on failure? johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-16 9:44 ` Johannes Berg @ 2026-10-16 9:47 ` Michael S. Tsirkin 2026-10-16 9:49 ` Johannes Berg 0 siblings, 1 reply; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-16 9:47 UTC (permalink / raw) To: Johannes Berg Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Fri, Oct 16, 2026 at 11:44:16AM +0200, Johannes Berg wrote: > On Fri, 2026-10-16 at 05:43 -0400, Michael S. Tsirkin wrote: > > > > > > > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > > > > > + if (virtio_net_hdr_to_skb(skb, vheader, > > > > > > + virtio_legacy_is_little_endian()) < 0) > > > > > > + return -1; > > > > > > return 0; > > > > > > > > > > > > > > > > Seems it'd be simpler to just > > > > > > > > > > return virtio_net_hdr_to_skb(...); > > > > > > > > > > As far as I can tell, the function only returns -EINVAL or 0 (never a > > > > > positive value), and the caller only cares about negative (error) or > > > > > positive (no checksum needed), so that'd be equivalent? > > > > > > > > > > johannes > > > > > > > > if raw_verify_header can safely return any errno - i did not want to > > > > audit it. > > > > > > You already used an LLM and still didn't want to? ;-) > > > > > > I checked briefly (by hand) and it seems fine, it's assigned to the > > > function pointer, which only has two callers on the _kern.c side. > > > > > > johannes > > > > > > header_check = vp->verify_header( > > mmsg_vector->msg_hdr.msg_iov->iov_base, > > skb, > > vp > > ); > > if (header_check < 0) { > > /* Overlay header failed to verify - discard. > > * We can actually keep this skb and reuse it, > > * but that will make the prep logic too > > * complex. > > */ > > dev_kfree_skb_irq(skb); > > vp->estats.rx_encaps_errors++; > > (*skbuff_vector) = NULL; > > mmsg_vector++; > > skbuff_vector++; > > continue; > > } > > if (header_check > 0) { > > vp->estats.rx_csum_offload_good++; > > skb->ip_summed = CHECKSUM_UNNECESSARY; > > } > > > > > > > > so any negative is fine, but if we return 1 then it will force to CHECKSUM_UNNECESSARY > > which I am not sure is what was intended? > > > Right, but virtio_net_hdr_to_skb() cannot return 1, only 0 on success > and -ERROR on failure? > > johannes After this patchset, it can, to signal slow path software checksum. I'm changing callers first so that changing the function itself does not break bisect. I should mention this in the commit log. -- MST ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-16 9:47 ` Michael S. Tsirkin @ 2026-10-16 9:49 ` Johannes Berg 2026-10-16 9:53 ` Michael S. Tsirkin 0 siblings, 1 reply; 13+ messages in thread From: Johannes Berg @ 2026-10-16 9:49 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Fri, 2026-10-16 at 05:47 -0400, Michael S. Tsirkin wrote: > > > > > Right, but virtio_net_hdr_to_skb() cannot return 1, only 0 on success > > and -ERROR on failure? > > After this patchset, it can, to signal slow path software checksum. > I'm changing callers first so that changing the function itself > does not break bisect. I should mention this in the commit log. Oh, OK, that I clearly missed, but also was only CC'ed on patches 2 and 3 :) johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-16 9:49 ` Johannes Berg @ 2026-10-16 9:53 ` Michael S. Tsirkin 0 siblings, 0 replies; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-16 9:53 UTC (permalink / raw) To: Johannes Berg Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, linux-um On Fri, Oct 16, 2026 at 11:49:14AM +0200, Johannes Berg wrote: > On Fri, 2026-10-16 at 05:47 -0400, Michael S. Tsirkin wrote: > > > > > > > Right, but virtio_net_hdr_to_skb() cannot return 1, only 0 on success > > > and -ERROR on failure? > > > > After this patchset, it can, to signal slow path software checksum. > > I'm changing callers first so that changing the function itself > > does not break bisect. I should mention this in the commit log. > > Oh, OK, that I clearly missed, but also was only CC'ed on patches 2 and > 3 :) > > johannes Right, I am trying not to spam too widely. Indeed, this is why commit log should explain. -- MST ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-15 12:41 ` [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures Michael S. Tsirkin 2026-10-15 12:47 ` Johannes Berg @ 2026-10-15 17:19 ` Willem de Bruijn 2026-10-16 9:40 ` Michael S. Tsirkin 1 sibling, 1 reply; 13+ messages in thread From: Willem de Bruijn @ 2026-10-15 17:19 UTC (permalink / raw) To: Michael S. Tsirkin, linux-kernel Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um Michael S. Tsirkin wrote: > virtio_net_hdr_to_skb() refuses packets whose checksum metadata > it cannot make sense of, but the vector raw transport ignored > its return value: rejected packets went on into the stack with > their metadata unvalidated. > > Propagate the failure. > > Assisted-by: LLM > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > arch/um/drivers/vector_transports.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > index e1fb2a76fdff..8b7548458663 100644 > --- a/arch/um/drivers/vector_transports.c > +++ b/arch/um/drivers/vector_transports.c > @@ -210,7 +210,9 @@ static int raw_verify_header( > return 1; > > skb_set_network_header(skb, ETH_HLEN); > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > + if (virtio_net_hdr_to_skb(skb, vheader, > + virtio_legacy_is_little_endian()) < 0) > + return -1; Should this return 1, like the above? Not sure whether the verify_header return value is well defined beyond just boolean. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures 2026-10-15 17:19 ` Willem de Bruijn @ 2026-10-16 9:40 ` Michael S. Tsirkin 0 siblings, 0 replies; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-16 9:40 UTC (permalink / raw) To: Willem de Bruijn Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um On Thu, Oct 15, 2026 at 01:19:22PM -0400, Willem de Bruijn wrote: > Michael S. Tsirkin wrote: > > virtio_net_hdr_to_skb() refuses packets whose checksum metadata > > it cannot make sense of, but the vector raw transport ignored > > its return value: rejected packets went on into the stack with > > their metadata unvalidated. > > > > Propagate the failure. > > > > Assisted-by: LLM > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > arch/um/drivers/vector_transports.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > > index e1fb2a76fdff..8b7548458663 100644 > > --- a/arch/um/drivers/vector_transports.c > > +++ b/arch/um/drivers/vector_transports.c > > @@ -210,7 +210,9 @@ static int raw_verify_header( > > return 1; > > > > skb_set_network_header(skb, ETH_HLEN); > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > + if (virtio_net_hdr_to_skb(skb, vheader, > > + virtio_legacy_is_little_endian()) < 0) > > + return -1; > > Should this return 1, like the above? > > Not sure whether the verify_header return value is well defined beyond > just boolean. header_check = vp->verify_header( vp->header_rxbuffer, skb, vp); if (header_check < 0) { dev_kfree_skb_irq(skb); vp->dev->stats.rx_dropped++; vp->estats.rx_encaps_errors++; return 0; } if (header_check > 0) { vp->estats.rx_csum_offload_good++; skb->ip_summed = CHECKSUM_UNNECESSARY; } apparently? maybe i misunderstand. -- MST ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v2 3/7] um: vector: trim skbs before verifying the header [not found] <cover.1792067836.git.mst@redhat.com> 2026-10-15 12:41 ` [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures Michael S. Tsirkin @ 2026-10-15 12:41 ` Michael S. Tsirkin 1 sibling, 0 replies; 13+ messages in thread From: Michael S. Tsirkin @ 2026-10-15 12:41 UTC (permalink / raw) To: linux-kernel Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Rusty Russell, Max Krasnyansky, Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um The vector receive path over-allocates the skb and only trims it to the received length after the transport header is verified. A follow up patch will complete the packet checksum in software within virtio_net_hdr_to_skb so we need the packet trimmed (otherwise the checksum would be computed incorrectly over the unreceived tail of the buffer). Trim first, so the import sees the packet as it was received. Assisted-by: LLM Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- arch/um/drivers/vector_kern.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c index 8a70b3a62553..1afbef78df72 100644 --- a/arch/um/drivers/vector_kern.c +++ b/arch/um/drivers/vector_kern.c @@ -865,6 +865,7 @@ static int vector_legacy_rx(struct vector_private *vp) if (skb != NULL) { if (pkt_len > vp->header_size) { + pskb_trim(skb, pkt_len - vp->rx_header_size); if (vp->header_size > 0) { header_check = vp->verify_header( vp->header_rxbuffer, skb, vp); @@ -879,7 +880,6 @@ static int vector_legacy_rx(struct vector_private *vp) skb->ip_summed = CHECKSUM_UNNECESSARY; } } - pskb_trim(skb, pkt_len - vp->rx_header_size); skb->protocol = eth_type_trans(skb, skb->dev); vp->dev->stats.rx_bytes += skb->len; vp->dev->stats.rx_packets++; @@ -981,6 +981,8 @@ static int vector_mmsg_rx(struct vector_private *vp, int budget) for (i = 0; i < packet_count; i++) { skb = (*skbuff_vector); if (mmsg_vector->msg_len > vp->header_size) { + pskb_trim(skb, + mmsg_vector->msg_len - vp->rx_header_size); if (vp->header_size > 0) { header_check = vp->verify_header( mmsg_vector->msg_hdr.msg_iov->iov_base, @@ -1005,8 +1007,6 @@ static int vector_mmsg_rx(struct vector_private *vp, int budget) skb->ip_summed = CHECKSUM_UNNECESSARY; } } - pskb_trim(skb, - mmsg_vector->msg_len - vp->rx_header_size); skb->protocol = eth_type_trans(skb, skb->dev); /* * We do not need to lock on updating stats here -- MST ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-16 9:53 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1792067836.git.mst@redhat.com>
2026-10-15 12:41 ` [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures Michael S. Tsirkin
2026-10-15 12:47 ` Johannes Berg
2026-10-15 12:52 ` Michael S. Tsirkin
2026-10-15 12:55 ` Johannes Berg
2026-10-16 9:37 ` Michael S. Tsirkin
2026-10-16 9:43 ` Michael S. Tsirkin
2026-10-16 9:44 ` Johannes Berg
2026-10-16 9:47 ` Michael S. Tsirkin
2026-10-16 9:49 ` Johannes Berg
2026-10-16 9:53 ` Michael S. Tsirkin
2026-10-15 17:19 ` Willem de Bruijn
2026-10-16 9:40 ` Michael S. Tsirkin
2026-10-15 12:41 ` [PATCH net-next v2 3/7] um: vector: trim skbs before verifying the header Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox