From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 221D8C433F5 for ; Thu, 6 Oct 2022 16:50:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231152AbiJFQuR (ORCPT ); Thu, 6 Oct 2022 12:50:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42724 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229636AbiJFQuO (ORCPT ); Thu, 6 Oct 2022 12:50:14 -0400 Received: from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com [IPv6:2607:f8b0:4864:20::22d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C194557E38 for ; Thu, 6 Oct 2022 09:50:12 -0700 (PDT) Received: by mail-oi1-x22d.google.com with SMTP id w70so2690401oie.2 for ; Thu, 06 Oct 2022 09:50:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=COxO5LnrIWzt/jw6mQyRzopKJeQXyul5IJodpgpQm8Y=; b=S6H/x/tg2ka0eQ2HCV0qdhVUwOlj2o7NtpPicRcoNc4nqQg640+CRyRqBxNZ5u9msy R9f/tBnMBcMJPZdSwDPjHMq7qGAE2FbRXkQgbC28foT+GhO/7kE9hvz/F3qy1Ts2q0Wc Fqpk/gPtgMXuZVod42ji11yA5dlhRfggAhsL4F3JoSrrdEf+epY0tmyZVjSRta62OIto i4sEJhDFXAjxX7dONm/7yq15JW8BA6C/ukkRfaXJqhKN/BYK+SeYUkd3kPbPldD2I5SO xQTUva7OihaDTG0Oa4gpBsvxJn8USgFNd62jrIYG9UxiAl6g4BL36QQR92feJHd0Ky0g CPEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=COxO5LnrIWzt/jw6mQyRzopKJeQXyul5IJodpgpQm8Y=; b=wkJPWisIi4Txw5ofKWxot9KepR8sEd4bTg84QHfU1IrjCCpdZDsUdqmA37SH1i1rHT r2iqFWw6ne9fBuDrm90ZHRmK0aLmGQo6OOOzByNLUfgHVbV6Vjvl1kcMX4tCCdW9SbER 37r1O1lUro3pT6Vkmi5/bXhFeG5+dsuFre63WiUVZ/aEMOL19jt2C3qgrjBpphBZAdJd z0Gq56FUZ4aeKFEy4MDn+Df6r6vS9jqJpDhrL/hn18GWpsrcih5eWKAmTtG/cR6k2xKQ uqCgSj1ZP+oWSo4Lp1XEXj/oYq2Q0ncQ+Tm296ulifkI8+r8VKmeYOhH/tMeAU8O5td/ K3ZA== X-Gm-Message-State: ACrzQf3TFoNRLJf0DYwHCEbzCh4V5JyNPzAjFoaRCjbhkxwvu5vw8QJN wH4OGIL4+uw2wN9+NVPrPJhpaqMGX8yrsnKDwTU= X-Google-Smtp-Source: AMsMyM58ZhZgde9ueV7y3NHPXDP+PmwB6yXdqsFnHLnMlhyCo7/h8Rd0ocdUDXmlecus06Q4ZutHpcywEMM+rffHHXk= X-Received: by 2002:a05:6808:15a2:b0:350:4f5c:1440 with SMTP id t34-20020a05680815a200b003504f5c1440mr5462264oiw.129.1665075012080; Thu, 06 Oct 2022 09:50:12 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Xin Long Date: Thu, 6 Oct 2022 12:49:24 -0400 Message-ID: Subject: Re: [PATCH net-next 3/3] net: sched: add helper support in act_ct To: Pablo Neira Ayuso Cc: network dev , dev@openvswitch.org, ovs-dev@openvswitch.org, davem@davemloft.net, kuba@kernel.org, Eric Dumazet , Paolo Abeni , Pravin B Shelar , Jamal Hadi Salim , Cong Wang , Jiri Pirko , Florian Westphal , Marcelo Ricardo Leitner , Davide Caratti , Oz Shlomo , Paul Blakey , Ilya Maximets , Eelco Chaudron Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Oct 6, 2022 at 10:11 AM Pablo Neira Ayuso wrote: > > On Tue, Oct 04, 2022 at 09:19:56PM -0400, Xin Long wrote: > [...] > > @@ -1119,6 +1135,22 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, > > if (err != NF_ACCEPT) > > goto drop; > > > > + if (commit && p->helper && !nfct_help(ct)) { > > + err = __nf_ct_try_assign_helper(ct, p->tmpl, GFP_ATOMIC); > > + if (err) > > + goto drop; > > + add_helper = true; > > + if (p->ct_action & TCA_CT_ACT_NAT && !nfct_seqadj(ct)) { > > + if (!nfct_seqadj_ext_add(ct)) > > You can only add ct extensions if ct is !nf_ct_is_confirmed(ct)), is > this guaranteed in this codepath? This is a good catch, the same issue exists on __nf_ct_try_assign_helper(), and also in __ovs_ct_lookup(). I could trigger the warning on OVS conntrack with the flow: table=0, in_port=veth1,tcp,tcp_dst=2121,ct_state=-trk actions=ct(commit, table=1) table=1, in_port=veth1,tcp,tcp_dst=2121,ct_state=+trk+new actions=ct(commit, alg=ftp),normal I will prepare a fix for ovs conntrack first. Thanks. > > > + return -EINVAL; > > + } > > + } > > + > > + if (nf_ct_is_confirmed(ct) ? ((!cached && !skip_add) || add_helper) : commit) { > > + if (nf_ct_helper(skb, family) != NF_ACCEPT) > > + goto drop; > > + } > > + > > if (commit) { > > tcf_ct_act_set_mark(ct, p->mark, p->mark_mask); > > tcf_ct_act_set_labels(ct, p->labels, p->labels_mask);