> -----Original Message-----
> From: Tom Herbert [mailto:t...@herbertland.com]
> Sent: Monday, September 12, 2016 4:23 PM
> To: Liang, Kan <kan.li...@intel.com>
> Cc: David S. Miller <da...@davemloft.net>; LKML <linux-
> ker...@vger.kernel.org>; Linux Kernel Network Developers
> <net...@vger.kernel.org>; Kirsher, Jeffrey T <jeffrey.t.kirs...@intel.com>;
> Ingo Molnar <mi...@redhat.com>; pet...@infradead.org; Alexey Kuznetsov
> <kuz...@ms2.inr.ac.ru>; James Morris <jmor...@namei.org>; Hideaki
> YOSHIFUJI <yoshf...@linux-ipv6.org>; Patrick McHardy <ka...@trash.net>;
> a...@linux-foundation.org; Kees Cook <keesc...@chromium.org>;
> v...@zeniv.linux.org.uk; gorcu...@openvz.org; John Stultz
> <john.stu...@linaro.org>; Alexander Duyck <adu...@mirantis.com>; Ben
> Hutchings <b...@decadent.org.uk>; David Decotigny <de...@googlers.com>;
> Florian Westphal <f...@strlen.de>; Alexander Duyck
> <alexander.du...@gmail.com>; Daniel Borkmann <dan...@iogearbox.net>;
> rdun...@infradead.org; Cong Wang <xiyou.wangc...@gmail.com>; Hannes
> Frederic Sowa <han...@stressinduktion.org>; Stephen Hemminger
> <step...@networkplumber.org>; Alexei Starovoitov
> <alexei.starovoi...@gmail.com>; Brandeburg, Jesse
> <jesse.brandeb...@intel.com>; Andi Kleen <a...@firstfloor.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.li...@intel.com> wrote:
> > From: Kan Liang <kan.li...@intel.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.li...@intel.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.

I will move the code to __netdev_pick_tx in next version.

Thanks,
Kan

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

Reply via email to