netdev - Re: [RFC V3 PATCH 18/26] net/netpolicy: set tx queues according to policy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Sep 2016 13:23:24 -0700
From:   Tom Herbert <tom@...bertland.com>
To:     "Liang, Kan" <kan.liang@...el.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        Ingo Molnar <mingo@...hat.com>, peterz@...radead.org,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        James Morris <jmorris@...ei.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Patrick McHardy <kaber@...sh.net>, akpm@...ux-foundation.org,
        Kees Cook <keescook@...omium.org>, viro@...iv.linux.org.uk,
        gorcunov@...nvz.org, John Stultz <john.stultz@...aro.org>,
        Alexander Duyck <aduyck@...antis.com>,
        Ben Hutchings <ben@...adent.org.uk>,
        David Decotigny <decot@...glers.com>,
        Florian Westphal <fw@...len.de>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>, rdunlap@...radead.org,
        Cong Wang <xiyou.wangcong@...il.com>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Andi Kleen <andi@...stfloor.org>
Subject: Re: [RFC V3 PATCH 18/26] net/netpolicy: set tx queues according to policy

On Mon, Sep 12, 2016 at 7:55 AM,  <kan.liang@...el.com> wrote:
> From: Kan Liang <kan.liang@...el.com>
>
> When the device tries to transmit a packet, netdev_pick_tx is called to
> find the available tx queues. If the net policy is applied, it picks up
> the assigned tx queue from net policy subsystem, and redirect the
> traffic to the assigned queue.
>
> Signed-off-by: Kan Liang <kan.liang@...el.com>
> ---
>  include/net/sock.h |  9 +++++++++
>  net/core/dev.c     | 20 ++++++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index e1e9e3d..ca97f35 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2280,4 +2280,13 @@ extern int sysctl_optmem_max;
>  extern __u32 sysctl_wmem_default;
>  extern __u32 sysctl_rmem_default;
>
> +/* Return netpolicy instance information from socket. */
> +static inline struct netpolicy_instance *netpolicy_find_instance(struct sock *sk)
> +{
> +#ifdef CONFIG_NETPOLICY
> +       if (is_net_policy_valid(sk->sk_netpolicy.policy))
> +               return &sk->sk_netpolicy;
> +#endif
> +       return NULL;
> +}
>  #endif /* _SOCK_H */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 34b5322..b9a8044 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3266,6 +3266,7 @@ struct netdev_queue *netdev_pick_tx(struct net_device *dev,
>                                     struct sk_buff *skb,
>                                     void *accel_priv)
>  {
> +       struct sock *sk = skb->sk;
>         int queue_index = 0;
>
>  #ifdef CONFIG_XPS
> @@ -3280,8 +3281,23 @@ struct netdev_queue *netdev_pick_tx(struct net_device *dev,
>                 if (ops->ndo_select_queue)
>                         queue_index = ops->ndo_select_queue(dev, skb, accel_priv,
>                                                             __netdev_pick_tx);
> -               else
> -                       queue_index = __netdev_pick_tx(dev, skb);
> +               else {
> +#ifdef CONFIG_NETPOLICY
> +                       struct netpolicy_instance *instance;
> +
> +                       queue_index = -1;
> +                       if (dev->netpolicy && sk) {
> +                               instance = netpolicy_find_instance(sk);
> +                               if (instance) {
> +                                       if (!instance->dev)
> +                                               instance->dev = dev;
> +                                       queue_index = netpolicy_pick_queue(instance, false);
> +                               }
> +                       }
> +                       if (queue_index < 0)
> +#endif

I doubt this produces the intended effect. Several drivers use
ndo_select_queue (such as mlx4) where there might do something special
for a few packets but end up called the default handler which
__netdev_pick_tx for most packets. So in such cases the netpolicy path
would be routinely bypassed. Maybe this code should be in
__netdev_pick_tx.

Tom

> +                               queue_index = __netdev_pick_tx(dev, skb);
> +               }
>
>                 if (!accel_priv)
>                         queue_index = netdev_cap_txqueue(dev, queue_index);
> --
> 2.5.5
>