Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191012071620.8595-1-zhiyuan2048@linux.alibaba.com>
In-Reply-To: <20191012071620.8595-1-zhiyuan2048@linux.alibaba.com>
From:   Cong Wang <xiyou.wangcong@gmail.com>
Date:   Mon, 14 Oct 2019 10:57:42 -0700
Message-ID: <CAM_iQpVkTb6Qf9J-PuXJoQTZa5ojN_oun64SMv9Kji7tZkxSyA@mail.gmail.com>
Subject: Re: [PATCH net] net: sched: act_mirred: drop skb's dst_entry in
 ingress redirection
To:     Zhiyuan Hou <zhiyuan2048@linux.alibaba.com>
Cc:     Jamal Hadi Salim <jhs@mojatatu.com>, Jiri Pirko <jiri@resnulli.us>,
        "David S . Miller" <davem@davemloft.net>,
        Linux Kernel Network Developers <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Oct 12, 2019 at 12:16 AM Zhiyuan Hou
<zhiyuan2048@linux.alibaba.com> wrote:
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 9ce073a05414..6108a64c0cd5 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -18,6 +18,7 @@
>  #include <linux/gfp.h>
>  #include <linux/if_arp.h>
>  #include <net/net_namespace.h>
> +#include <net/dst.h>
>  #include <net/netlink.h>
>  #include <net/pkt_sched.h>
>  #include <net/pkt_cls.h>
> @@ -298,8 +299,10 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
>
>         if (!want_ingress)
>                 err = dev_queue_xmit(skb2);
> -       else
> +       else {
> +               skb_dst_drop(skb2);
>                 err = netif_receive_skb(skb2);
> +       }

Good catch!

I don't want to be picky, but it seems this is only needed
when redirecting from egress to ingress, right? That is,
ingress to ingress, or ingress to egress is okay? If not,
please fix all the cases while you are on it?

Thanks.