Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Dynamically disable/enable virtio_net LRO/CSUM #2433

Closed
Vesnica opened this issue Jan 28, 2021 · 10 comments
Closed

[Feature Request] Dynamically disable/enable virtio_net LRO/CSUM #2433

Vesnica opened this issue Jan 28, 2021 · 10 comments
Assignees
Labels
Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: WIP Indicates that an issue is currently being worked on or triaged Type: Enhancement Indicates new feature requests

Comments

@Vesnica
Copy link

Vesnica commented Jan 28, 2021

Feature Request

I'm trying to bind a XDP program to eth0 in firecracker vm. like this:
ip -force link set dev eth0 xdpdrv obj bpf_xdp.o sec from-netdev

But it failed with message:
Error: virtio_net: Can't set XDP while host is implementing LRO/CSUM, disable LRO/CSUM first

ethtool produce the following message:

root@locahost:~# ethtool -k eth0
Features for eth0:
rx-checksumming: on [fixed]
tx-checksumming: on
        tx-checksum-ipv4: off [fixed]
        tx-checksum-ip-generic: on
        tx-checksum-ipv6: off [fixed]
        tx-checksum-fcoe-crc: off [fixed]
        tx-checksum-sctp: off [fixed]
scatter-gather: on
        tx-scatter-gather: on
        tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: on
        tx-tcp-segmentation: on
        tx-tcp-ecn-segmentation: off [fixed]
        tx-tcp-mangleid-segmentation: off
        tx-tcp6-segmentation: off [fixed]
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: on [fixed]
rx-vlan-offload: off [fixed]
tx-vlan-offload: off [fixed]
ntuple-filters: off [fixed]
receive-hashing: off [fixed]
highdma: on [fixed]
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: on [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-gre-csum-segmentation: off [fixed]
tx-ipxip4-segmentation: off [fixed]
tx-ipxip6-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
tx-udp_tnl-csum-segmentation: off [fixed]
tx-gso-partial: off [fixed]
tx-tunnel-remcsum-segmentation: off [fixed]
tx-sctp-segmentation: off [fixed]
tx-esp-segmentation: off [fixed]
tx-udp-segmentation: off [fixed]
tx-gso-list: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: off
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
l2-fwd-offload: off [fixed]
hw-tc-offload: off [fixed]
esp-hw-offload: off [fixed]
esp-tx-csum-hw-offload: off [fixed]
rx-udp_tunnel-port-offload: off [fixed]
tls-hw-tx-offload: off [fixed]
tls-hw-rx-offload: off [fixed]
rx-gro-hw: off [fixed]
tls-hw-record: off [fixed]
rx-gro-list: off
macsec-hw-offload: off [fixed]

Because rx-checksumming and large-receive-offload are fixed,so I can't turn it off.

This prohibits the ebpf application in firecracker vm.

Describe the desired solution

Modify the net device code to make dynamically enable/disable features possible.

Checks

  • [✔] Have you searched the Firecracker Issues database for similar requests?
  • [✔] Have you read all the existing relevant Firecracker documentation?
  • [✔] Have you read and understood Firecracker's core tenets?
@sandreim
Copy link
Contributor

Hi @Vesnica. I understand that this a limitation for your usecase. We'll get back to you once we understand what are the options to enable it. Are you interested in contributing the code changes?

@Vesnica
Copy link
Author

Vesnica commented Jan 29, 2021

Unfortunately, I don’t know much about Rust or network device drivers.

The demand of this feature is not urgent at the moment, you can mark it as a low priority.

@sandreim sandreim added Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Feature: IO Virtualization labels Jan 29, 2021
@aaglenn
Copy link

aaglenn commented Apr 13, 2021

in looking to add support for veth pairs, I came across this issue and believe I've "found" the responsible code: https://github.com/firecracker-microvm/firecracker/blob/master/src/devices/src/virtio/net/device.rs#L33

this is a somewhat high priority for myself; and though I have no experience writing Rust and about 30 minutes familiarity with the firecracker codebase, I have no fear attempting to submit mergeable PR's (especially if there is anyone willing to assist/answer questions that arise)

@serban300
Copy link
Contributor

Hi @aaglenn ! Indeed, that is the net device source file. The code that sets the virtio flags is here.

I was curious if you have a specific solution in mind. Modifying the flags or exposing them as API parameters should be easy. But the question is whether we can do this in a generic way. Exposing specific virtio flags through the API doesn't look like a feasible solution.

@chenhengqi
Copy link

For purpose of attaching a XDP object to network device, xdpgeneric mode would work.

@historyliao
Copy link

@chenhengqi But xdpgeneric is a test mode with poor performance.

@cperciva
Copy link
Contributor

I believe this is actually two bugs in one:

  1. Firecracker doesn't implement resetting virtio devices ([Feature Request] support for resetting virtio devices #3074).
  2. Firecracker ignores guests disabling virtio-net features ([Bug] virtio-net: Guest can't NACK LRO #3905).

"Dynamically disabling virtio_net features" really means "reset the device and negotiate a new feature set", but neither part of that works.

@jeromegn
Copy link
Contributor

I've forked and added some flags to the virtio device, yet the features are not enabled when looking at them via ethtool -k. Commenting here instead of creating a new issue because it sounds related...

I was attempting to add tx ipv6 checksum:

        let mut avail_features = 1 << VIRTIO_NET_F_GUEST_CSUM
            | 1 << VIRTIO_NET_F_CSUM
            | 1 << VIRTIO_NET_F_GUEST_TSO4
            | 1 << VIRTIO_NET_F_GUEST_TSO6
            | 1 << VIRTIO_NET_F_GUEST_UFO
            | 1 << VIRTIO_NET_F_HOST_TSO4
            | 1 << VIRTIO_NET_F_HOST_TSO6
            | 1 << VIRTIO_NET_F_HOST_UFO
            | 1 << VIRTIO_F_VERSION_1;

@ShadowCurse ShadowCurse added the Good first issue Indicates a good issue for first-time contributors label Nov 20, 2023
@ShadowCurse ShadowCurse assigned bchalios and unassigned alsrdn Nov 20, 2023
@ShadowCurse
Copy link
Contributor

Hi all, sorry for the delay, unfortunately we were not able to make progress regarding this issue.
We are still tracking the problem and will provide an update once we will have more data to share.

@zulinx86 zulinx86 added the Status: WIP Indicates that an issue is currently being worked on or triaged label Jun 20, 2024
@roypat
Copy link
Contributor

roypat commented Nov 13, 2024

Hey all,
as @cperciva points out, this really is a combination of two feature requests: Resetting virtio devices, and being able to NACK virtio features. We've since implemented the latter of these in #4680 (see also #3905), leaving us with only reset functionality missing. We're tracking that in #3074 though, so I'll go ahead and close this issue to avoid duplicated issues. Thanks!

@roypat roypat closed this as completed Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: WIP Indicates that an issue is currently being worked on or triaged Type: Enhancement Indicates new feature requests
Projects
None yet
Development

No branches or pull requests