Page MenuHomeFreeBSD

netfront: fix the support for disabling LRO at boot time
ClosedPublic

Authored by dfr on Aug 13 2023, 10:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 29, 3:57 PM
Unknown Object (File)
Fri, Apr 26, 5:37 PM
Unknown Object (File)
Mon, Apr 8, 1:23 PM
Unknown Object (File)
Dec 22 2023, 5:01 PM
Unknown Object (File)
Dec 20 2023, 7:25 AM
Unknown Object (File)
Dec 17 2023, 7:49 AM
Unknown Object (File)
Dec 10 2023, 8:30 PM
Unknown Object (File)
Nov 23 2023, 11:55 AM

Details

Summary

The driver has a tunable hw.xn.enable_lro which is intended to control
whether LRO is enabled. This is currently non-functional - even if its
set to zero, the driver still requests LRO support from the backend.
This change fixes the feature so that if enable_lro is set to zero, LRO
no longer appears in the interface capabilities and LRO is not requested
from the backend.

PR: 273046
MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dfr requested review of this revision.Aug 13 2023, 10:25 AM

This looks plausible. Likely it mostly amounts to a debugging tool since you would basically never want this in anything approximating a real world situation.

This looks plausible. Likely it mostly amounts to a debugging tool since you would basically never want this in anything approximating a real world situation.

Whether or not its a debugging tool, its currently broken. This change allows a user to disable LRO at boot time which (for me) is a useful workaround for using this driver in VMs with forwarding enabled.

I will look in more detail on Monday when I'm back from PTO, but overall I agree it should be fixed. I will also look into possible solutions to detect when forwarding is enabled and disable LRO automatically.

Would it be simpler to not set IFCAP_LRO in the if_setcapenable() call on create_netdev() if xn_enable_lro is false? I think that way we would avoid exposing feature-gso-tcpv4 on xenstore and you won't need to modify talk_to_backend().

sys/dev/xen/netfront/netfront.c
2080

I think strictly speaking you shouldn't need to modify this, as setting the feature in available capabilities is fine, but it must not be set in capenable.

Rebased and simplified based on review feedback.

Would it be simpler to not set IFCAP_LRO in the if_setcapenable() call on create_netdev() if xn_enable_lro is false? I think that way we would avoid exposing feature-gso-tcpv4 on xenstore and you won't need to modify talk_to_backend().

Great idea - that worked really well and simplifies the diff.

dfr marked an inline comment as done.Sep 11 2023, 10:14 AM

Are you fine with committing it yourself? Might be worth requesting for backport to the releng 14.0 branch, as it's a low risk fix IMO.

This revision is now accepted and ready to land.Oct 3 2023, 10:43 AM

I'll land it shortly - I also plan to merge it to stable/14 and stable/13. I'll see what re@ thinks about releng/14.0.