lagg: re-check port caps after bringing up a port
ClosedPublic
Actions

Authored by gallatin on Mon, Jul 6, 6:55 PM.

Details

Reviewers

sumit.saxena_broadcom.com
markj
zlei
glebius

Commits

rG1cce5ef05d7b: lagg: re-check port caps after bringing up a port

Summary

Some nic drivers (including iflib) do not initialize if_hwassist until after the interface is brought up.
If a lagg member is included in a lagg when its not yet been brought up, that will cause lagg to see
if_hwassist=0 and will disable all checksum offload, etc, on the interface. This is almost impossible
to debug without kgdb or dtrace, as ifconfig does not surface if_hwassist.

Fix this by re-calculating lagg caps (including if_hwassist) after adding a port.

I encountered this problem when I had a commented-out if_foo1=up entry in my rc.conf that i neglected
to uncomment when I was re-configuring a lagg.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

gallatin created this revision.Mon, Jul 6, 6:55 PM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptMon, Jul 6, 6:55 PM

gallatin requested review of this revision.Mon, Jul 6, 6:55 PM

markj accepted this revision.Tue, Jul 7, 2:22 PM

markj added inline comments.

sys/net/if_lagg.c
1313	Maybe this should be done in lagg_if_updown()?

This revision is now accepted and ready to land.Tue, Jul 7, 2:22 PM

Looks good to me.

sys/net/if_lagg.c
1313	Typically `hwassist` will be updated only when the interfaces being brought up. After the interface is brought down, it shall stop transmitting any packets ( both outbound and inbound ) so it appears to be not useful to update lagg(4)'s or port interfaces' `hwassist` .

gallatin marked 2 inline comments as done.Wed, Jul 8, 12:23 PM

gallatin added inline comments.

sys/net/if_lagg.c
1313	As @zlei points out, lagg_if_updown() handles both up/down events and is called from just lagg_stop() (on the way down) and lagg_init_locked() (on the way up). It looks like all the stuff we do on the way up is in lagg_init_locked(), so I think it makes the most sense to keep it here..