On 06/04/2023 09:47, Robin Jarry wrote:
Kevin Traynor, Mar 23, 2023 at 15:27:
Hi Robin,

Regarding having both features enabled, I think it's an issue that it's
chronological based if they are enabled while running. It introduces
another variable that might confuse things.

For example, the operation could be changed from cp-proto to hw-offload
on a port by restarting OVS, which would probably be unexpected by a
user. I mentioned it while chatting to Ilya and he agreed that same
state in ovsdb should mean same state in ovs-vswitchd.

So that would mean having a binary priority between the two features and
   removing one if the higher priority one was later enabled (either
globally or per-port?).

Whatever the co-existance (or not) is, I think it's better to resolve it
in mail first to avoid you having to rework code over again. I don't
think it needs to be super-smart as these are experimental features,
just needs to be consistent and clearly documented for the user.

Code wise, I've tested previous versions and I think the code is in
pretty good shape overall. I'll do another pass review/testing when the
hwol/cp-prot prio is resolved.

thanks,
Kevin.

Hi Kevin,

sorry not to have replied earlier, I got caught up in other issues :)

I agree that having a deterministic priority between rte flow offload
and control plane protection is a must have. However, I am not sure how
to implement it in the current state of things.

The main issue is that cp-protection is dpdk specific whereas hw-offload
is in the abstract netdev layer. There is no way to check the state of
cp-protection from netdev-offload.c. Maybe I could expose a minimal
generic API in netdev.h to determine if hw-offload can be enabled for
a specific device or not. And implement it for dpdk, based on the value
of cp-protection.

What do you think?


In netdev-dpdk netdev_dpdk_flow_api_supported(), there is already code to check to see if cp-proto is enabled on a port when hw-offload gets set. There is also code to unconfigure the cp-proto flows in case the user removed the option.

So rather than printing the mutual exclusive warning innetdev_dpdk_flow_api_supported(), could you just trigger an unconfigure of the cp-proto?

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to