lagg: fix ifioctl after SIOCSIFCAPNV

[gallatin]

Summary

Lagg was broken by SIOCSIFCAPNV when all underlying devices support SIOCSIFCAPNV. This patch updates lagg to work with SIOCSIFCAPNV and if_capabilities2

Note: I wasn't sure of the purpose of SIOCGIFCAPNV. I followed the mce model of returning 0. Perhaps I should iterate over all ports and ensure each driver returns 0?

Test Plan

ifconfig lagg0 with underlying devices, such as Mellanox CX6-DX, which support SIOCSIFCAPNV. Ensure ifconfig doesn't present an error like:

# ifconfig lagg0
lagg0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
ifconfig: ioctl (SIOCGIFCAPNV): Invalid argument

Ensure caps from both if_capabilities2 (rxtls) and if_capabilities (tso) can be enabled, and settings are propagated between lagg0 and lagg ports.

[• hselasky]

sys/net/if_lagg.c

2045

Only if SIOCSIFCAPNV fails you fallback to SIOCSIFCAP ?

[kib]

After the fix fo lagg_setcaps(), it probably fine for now, since only mce(4) uses CAPNV at the moment, and only for reqcap2/if_capabilities2. When more advanced uses of CAPNV appear, for instance, the IPSEC support with capabilities reporting the supported modes and algorithms is implemented, this would be not enough.

sys/net/if_lagg.c

2045

It should be checked for IFCAP_NV before calling SIOCSIFCAPNV, and SIOCSIFCAP should not be called at all if cmd is SIOCSIFCAPNV. Note that ioctl_data is different for SIOCSIFCAP vs. SIOCSIFCAPNV.

[gallatin]

sys/net/if_lagg.c

2045

I was on the fence about this. I did it this way because I didn't trust random drivers to properly return an error for SIOCSIFCAPNV, but I didn't think to check for IFCAP_NV. I'll update it to do this. Thank you!

[• hselasky]

sys/net/if_lagg.c

2045

Checking for IFCAP_NV is the right thing to do, like Konstantin suggested.

[gallatin]

• Updated lagg_setcaps() to use IFCAP_NV SIOCSIFCAPNV on the lagg port to decide if it should use the SIOCSIFCAPNV or the SIOCSIFCAP ioctl method

• Updated lagg_setcaps() to return if both if_capenable2 matches cap2 (as well as if_capenable matching cap)

[• hselasky]

sys/net/if_lagg.c

677–680

I think there is a small technical chance that all caps may be set.

So I think we may want a boolean there to check if the CK_SLIST is empty.

bool any;

any = false;

CK_SLIST...
any = true;

if (any == false) {
ena = 0;
ena2 = 0;
}

706

Ditto.

[• hselasky]

sys/net/if_lagg.c

677–680

Or maybe just:

any = CK_SLIST_FIRST(...) != NULL;

[gallatin]

Since all bits may be set, use the fact that the list is empty, rather than all bits remaining set, to zero caps and enables, as suggested by Hans

[• hselasky]

Looks good.

Probably you should check:

if_capenable & IFCAP_NV

Instead of:

if ((lp->lp_ifp->if_capabilities & IFCAP_NV) != 0) {

But I see no reason why IFCAP_NV should be turned off, so let's keep it this way for now.

[kib]

sys/net/if_lagg.c

2046

You also need to set drv_ioctl_data.nvcap to something reasonable.

The contract there was to have nvcap to include all bits from reqcap/reqcap2 as boolean members of the nvcap nvlist. For now I think you can paper over it with setting nvcap to NULL.

[gallatin]

• Null'ed drv_ioctl_data.nvcap as suggested by @kib