On 20/07/2020 18:21, Jakub Kicinski wrote: > On Mon, 20 Jul 2020 12:45:54 +0100 Edward Cree wrote: >> I think I'd prefer to keep the switch() that explicitlychecks >> for UDP_TUNNEL_TYPE_GENEVE; even though the infrastructure >> makes sure it won't ever not be, I'd still feel more comfortable >> that way. But it's up to you. > > To me the motivation of expressing capabilities is for the core > to be able to do the necessary checking (and make more intelligent > decisions). All the drivers I've converted make the assumption they > won't see tunnel types they don't support.
Like I say, up to you. It's not how I'd write it but if that's how you're doing all the drivers then consistency is probably good. >> Could we not keep a 'valid'/'used' flag in the table, used in >> roughly the same way we were checking count != 0? > > How about we do the !port check in efx_ef10_udp_tnl_has_port()? > > Per-entry valid / used flag seems a little wasteful. > > Another option is to have a reserved tunnel type for invalid / unused. Reserved tunnel type seems best to me. (sfc generally uses all-ones values for reserved, so this would be 0xffff.) Alternatively you could change it to store an enum efx_encap_type in the table (see filter.h), and move the conversion to MCDI values to efx_ef10_set_udp_tnl_ports(), since that has a defined NONE value. But that means converting twice (from udp_parseable_tunnel_type to efx_encap_type and then again to MCDI) which isn't the prettiest. -ed
