On Fri, Mar 29, 2024 at 12:26:11AM +0100, Ilya Maximets wrote:
> On 3/22/24 14:54, Eric Garver wrote:
[..]
> > @@ -1649,8 +1693,8 @@ check_support(struct dpif_backer *backer)
> > backer->rt_support.max_hash_alg = check_max_dp_hash_alg(backer);
> > backer->rt_support.check_pkt_len = check_check_pkt_len(backer);
> > backer->rt_support.ct_timeout = check_ct_timeout_policy(backer);
> > - backer->rt_support.explicit_drop_action =
> > - dpif_supports_explicit_drop_action(backer->dpif);
> > + atomic_store_relaxed(&backer->rt_support.explicit_drop_action,
> > + check_drop_action(backer));
> > backer->rt_support.lb_output_action =
> > dpif_supports_lb_output_action(backer->dpif);
> > backer->rt_support.ct_zero_snat = dpif_supports_ct_zero_snat(backer);
> > @@ -1667,6 +1711,28 @@ 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)
> > + && !check_drop_action(backer)) {
>
> Shouldn't last two conditions be || instad of && ?
Assuming you mean:
if (explicit_drop_action &&
(!dpif_may_support_explicit_drop_action(backer->dpif) ||
!check_drop_action(backer))) {
Then I don't think so. This function is periodically called from
type_run(). If we use || here, then the usual case is that
dpif_may_support_explicit_drop_action() will return true, i.e.
hw-offload=false. That would force check_drop_action() to be called.
Basically we want to avoid calling check_drop_action() by guarding it
with the much cheaper dpif_may_support_explicit_drop_action().
> > + atomic_store_relaxed(&backer->rt_support.explicit_drop_action,
> > false);
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
[..]
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
