On 2 Apr 2025, at 9:32, Roi Dayan wrote:
> Calling dpif_probe_feature() should return if supported or not
> without relation to the dpif name.
> Remove the redundant recheck of drop support.
> TC is supporting drop action from the following commit.
> [1] 002682727e6e ("netdev-offload-tc: Add drop action support.")
Hi Roi,
This is for drop support, not explicit drop support with reasoning, so just
removing this code will not work. Also, this patch is failing quite some
regressions.
//Eelco
PS: It might be a good idea to enable testing on your local GitHub repo and
push your patches there first. That way, you can make sure your changes are
tested before sending them upstream!
> Signed-off-by: Roi Dayan <r...@nvidia.com>
> Reviewed-by: Eli Britstein <el...@nvidia.com>
> ---
> lib/dpif.c | 7 -------
> lib/dpif.h | 1 -
> ofproto/ofproto-dpif.c | 31 ++-----------------------------
> 3 files changed, 2 insertions(+), 37 deletions(-)
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index c22a47c3c15e..58ef90a5ded0 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1936,13 +1936,6 @@ dpif_supports_tnl_push_pop(const struct dpif *dpif)
> return dpif_is_netdev(dpif);
> }
>
> -bool
> -dpif_may_support_explicit_drop_action(const struct dpif *dpif)
> -{
> - /* TC does not support offloading this action. */
> - return dpif_is_netdev(dpif) || !netdev_is_flow_api_enabled();
> -}
> -
> bool
> dpif_supports_lb_output_action(const struct dpif *dpif)
> {
> diff --git a/lib/dpif.h b/lib/dpif.h
> index a764e8a592bd..8dc5da45b3ef 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -940,7 +940,6 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> odp_port_t port_no,
>
> char *dpif_get_dp_version(const struct dpif *);
> bool dpif_supports_tnl_push_pop(const struct dpif *);
> -bool dpif_may_support_explicit_drop_action(const struct dpif *);
> bool dpif_synced_dp_layers(struct dpif *);
>
> /* Log functions. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 7637d93de73e..b8b32b5778e7 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -223,7 +223,6 @@ static void ct_zone_config_init(struct dpif_backer
> *backer);
> static void ct_zone_config_uninit(struct dpif_backer *backer);
> static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer);
> static void ct_zone_limits_commit(struct dpif_backer *backer);
> -static bool recheck_support_explicit_drop_action(struct dpif_backer *backer);
>
> static inline struct ofproto_dpif *
> ofproto_dpif_cast(const struct ofproto *ofproto)
> @@ -394,10 +393,6 @@ type_run(const char *type)
> udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
> }
>
> - if (recheck_support_explicit_drop_action(backer)) {
> - backer->need_revalidate = REV_RECONFIGURE;
> - }
> -
> if (backer->need_revalidate) {
> struct ofproto_dpif *ofproto;
> struct simap_node *node;
> @@ -1420,8 +1415,8 @@ check_drop_action(struct dpif_backer *backer)
> ofpbuf_use_stack(&actions, &actbuf, sizeof actbuf);
> nl_msg_put_u32(&actions, OVS_ACTION_ATTR_DROP, XLATE_OK);
>
> - supported = dpif_may_support_explicit_drop_action(backer->dpif) &&
> - dpif_probe_feature(backer->dpif, "drop", &key, &actions,
> NULL);
> + supported = dpif_probe_feature(backer->dpif, "drop", &key, &actions,
> + NULL);
>
> VLOG_INFO("%s: Datapath %s explicit drop action",
> dpif_name(backer->dpif),
> @@ -1757,28 +1752,6 @@ check_support(struct dpif_backer *backer)
> backer->rt_support.odp.nd_ext = check_nd_extensions(backer);
> }
>
> -/* TC does not support offloading the explicit drop action. As such we need
> to
> - * re-probe the datapath if hw-offload has been modified.
> - * Note: We don't support true --> false transition as that requires a
> restart.
> - * See netdev_set_flow_api_enabled(). */
> -static bool
> -recheck_support_explicit_drop_action(struct dpif_backer *backer)
> -{
> - bool explicit_drop_action;
> -
> - atomic_read_relaxed(&backer->rt_support.explicit_drop_action,
> - &explicit_drop_action);
> -
> - if (explicit_drop_action
> - && !dpif_may_support_explicit_drop_action(backer->dpif)) {
> - ovs_assert(!check_drop_action(backer));
> - atomic_store_relaxed(&backer->rt_support.explicit_drop_action,
> false);
> - return true;
> - }
> -
> - return false;
> -}
> -
> static int
> construct(struct ofproto *ofproto_)
> {
> --
> 2.21.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
