On 11/27/25 12:44 PM, Yael Chemla via dev wrote:
> Currently, the kernel capability probe for TCA_FLOWER_KEY_ENC_FLAGS
> does not take into account whether hardware offload is enabled. This
> can lead to incorrect detection of encapsulation flag support and
> failed offload attempts on devices lacking proper hardware support.
>
> This patch uses a two-step probe to validate hardware support for
> ENC_FLAGS:
> (1) A global skip_hw probe checks whether the kernel recognizes
> TCA_FLOWER_KEY_ENC_FLAGS.
> (2) Per device, if it’s a tunnel and hw offload is enabled, we
> install a temporary flower rule and read it back with tc_get_flower to
> confirm offloaded_state == IN_HW. This checks whether ENC_FLAGS can
> actually be offloaded to hardware.
> Results are cached per device in the new netdev_tc_data structure, and
> we skip probing for system/non-tunnel devices. This avoids false
> positives where the global skip_hw probe succeeds but the device
> supports the key only in software, and prevents unnecessary probes and
> offload attempts on unsupported devices.
>
> ENC flags probing is serialized by an atomic state machine in
> netdev_tc_data. A call to probe_enc_flags_support() uses
> atomic_compare_exchange_strong_relaxed() to transition
> enc_flags_probe_state from OVS_PROBE_UNINIT to OVS_PROBE_STARTED;
> only the thread that wins this transition executes the probe, while
> others observe STARTED/DONE and skip. This guarantees at most one
> concurrent probe per netdev and avoids redundant work.
> Acquire–release ordering ensures both the “started” state and
> the final supported/unsupported result are correctly published to
> readers such as netdev_tc_get_enc_flags_support().
>
> v2:
> - A small atomic state machine (OVS_PROBE_ENC_FLAGS_*) and a CAS gate
> ensure only one thread runs probe_enc_flags_support() per netdev
> - relocate tc_data cleanup to uninit_flow_api
> - Ensured qdisc creation/deletion only when necessary.
>
> Fixes: 3f7af5233a29 ("netdev-offload-tc: Check if TCA_FLOWER_KEY_ENC_FLAGS is
> supported.")
> Reviewed-by: Jianbo Liu <[email protected]>
> Signed-off-by: Yael Chemla <[email protected]>
> ---
Hi, Yael. Thanks for the patch. Though it feels like we're adding
a lot of infrastructure for nothing. As I suggested in the previous
threads about this issue, it's better if we just stop matching on
these flags altogether when they are not necessary. Ignoring them
when they are necessary is not really a good option. If we stop
matching on df and csum, we may also then cleanup the offload-tc
code and not even probe for flags at all, as any flags that are set
will be necessary, so we may just let the request to fail if some
of them are not supported.
I posted a patch to stop matching n df and csum here:
https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
