On 8/20/2019 8:50 AM, Pravin Shelar wrote: > On Mon, Aug 19, 2019 at 10:42 AM Marcelo Ricardo Leitner > <marcelo.leit...@gmail.com> wrote: >> On Sun, Aug 18, 2019 at 07:00:59PM +0300, Paul Blakey wrote: >>> What do you guys say about the following diff on top of the last one? >>> Use static key, and also have OVS_DP_CMD_SET command probe/enable the >>> feature. >>> >>> This will allow userspace to probe the feature, and selectivly enable it >>> via the >>> OVS_DP_CMD_SET command. >> I'm not convinced yet that we need something like this. Been >> wondering, skb_ext_find() below is not that expensive if not in use. >> It's just a bit check and that's it, it returns NULL. >> >> And drivers will only be setting this if they have tc-offloading >> enabled (assuming they won't be seeing it for chain 0 all the time). >> On which case, with tc offloading, we need this in order to work >> properly. >> >> Is the bit checking really that worrysome? >> > Point is this would be completely unnecessary check for software only > cases, that is what static key is used for, when you have a feature in > datapath that is not used by majority of users. So I do not see any > downside of having this static key.
Also it's good that I can now probe kernel support via OVS_DP_CMD_SET. Since user can disable the kernel support or even use a different version of the datapath module, it would silently bug on misses from tc. Now I can set the recirc sharing feature in user_feature via SET op, and check the returned new user_features or an error code. If no error, and user_features contains the flag (won't be there for older DPs that won't return error), then DP supports this feature and kernel config is enabled.
