[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 5 May 2021 09:39:45 -0700
From: mleitner@...hat.com
To: Paul Blakey <paulb@...dia.com>
Cc: Jamal Hadi Salim <jhs@...atatu.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org,
Oz Shlomo <ozsh@...dia.com>, Roi Dayan <roid@...dia.com>,
Jiri Pirko <jiri@...dia.com>,
Saeed Mahameed <saeedm@...dia.com>,
Vlad Buslov <vladbu@...dia.com>
Subject: Re: [PATCH net-next] net/sched: act_ct: Offload connections with
commit action
On Wed, May 05, 2021 at 01:56:06PM +0300, Paul Blakey wrote:
> Currently established connections are not offloaded if the filter has a
> "ct commit" action. This behavior will not offload connections of the
> following scenario:
>
> $ tc_filter add dev $DEV ingress protocol ip prio 1 flower \
> ct_state -trk \
> action ct commit action goto chain 1
>
> $ tc_filter add dev $DEV ingress protocol ip chain 1 prio 1 flower \
> action mirred egress redirect dev $DEV2
>
> $ tc_filter add dev $DEV2 ingress protocol ip prio 1 flower \
> action ct commit action goto chain 1
>
> $ tc_filter add dev $DEV2 ingress protocol ip prio 1 chain 1 flower \
> ct_state +trk+est \
> action mirred egress redirect dev $DEV
>
> Offload established connections, regardless of the commit flag.
>
> Reviewed-by: Oz Shlomo <ozsh@...dia.com>
> Reviewed-by: Jiri Pirko <jiri@...dia.com>
> Signed-off-by: Paul Blakey <paulb@...dia.com>
> ---
> net/sched/act_ct.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index ec7a1c438df9..b1473a1aecdd 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -984,7 +984,7 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
> */
> cached = tcf_ct_skb_nfct_cached(net, skb, p->zone, force);
> if (!cached) {
> - if (!commit && tcf_ct_flow_table_lookup(p, skb, family)) {
> + if (tcf_ct_flow_table_lookup(p, skb, family)) {
Took me a while to realize that a zone check is not needed here
because when committing to a different zone it will check the new
flowtable here already. Otherwise, for commits, the zone update was
enforced in the few lines below.
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> skip_add = true;
> goto do_nat;
> }
> @@ -1022,10 +1022,11 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
> * even if the connection is already confirmed.
> */
> nf_conntrack_confirm(skb);
> - } else if (!skip_add) {
> - tcf_ct_flow_table_process_conn(p->ct_ft, ct, ctinfo);
> }
>
> + if (!skip_add)
> + tcf_ct_flow_table_process_conn(p->ct_ft, ct, ctinfo);
> +
> out_push:
> skb_push_rcsum(skb, nh_ofs);
>
> --
> 2.30.1
>
Powered by blists - more mailing lists