netdev - Re: [PATCH net-next 5/5] veth: implement ndo_set_rx

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]

Message-ID: <1456305454.5436.28.camel@redhat.com>
Date:	Wed, 24 Feb 2016 10:17:34 +0100
From:	Paolo Abeni <pabeni@...hat.com>
To:	pravin shelar <pshelar@....org>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Stephen Hemminger <stephen@...workplumber.org>,
	Pravin Shelar <pshelar@...ira.com>,
	Jesse Gross <jesse@...nel.org>,
	Flavio Leitner <fbl@...close.org>,
	Hannes Frederic Sowa <hannes@...essinduktion.org>
Subject: Re: [PATCH net-next 5/5] veth: implement ndo_set_rx_headroom

On Tue, 2016-02-23 at 11:21 -0800, pravin shelar wrote:
> On Tue, Feb 23, 2016 at 4:53 AM, Paolo Abeni <pabeni@...hat.com> wrote:
> > The rx headroom for veth dev is the peer device needed_headroom.
> > Avoid ping-pong updates setting the private flag IFF_PHONY_HEADROOM.
> >
> > This avoids skb head reallocation when forwarding from a veth dev
> > towards a device adding some kind of encapsulation.
> >
> > When forwarding frames below the MTU size towards a vxlan device,
> > this gives about 10% performance speed-up when OVS is used to connect
> > the veth and the vxlan device and a little more when using a
> > plain Linux bridge.
> >
> > Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> > ---
> >  drivers/net/veth.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> > index ba21d07..4f30a6a 100644
> > --- a/drivers/net/veth.c
> > +++ b/drivers/net/veth.c
> > @@ -35,6 +35,7 @@ struct pcpu_vstats {
> >  struct veth_priv {
> >         struct net_device __rcu *peer;
> >         atomic64_t              dropped;
> > +       unsigned                requested_headroom;
> >  };
> >
> >  /*
> > @@ -271,6 +272,29 @@ static int veth_get_iflink(const struct net_device *dev)
> >         return iflink;
> >  }
> >
> > +static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
> > +{
> > +       struct veth_priv *peer_priv, *priv = netdev_priv(dev);
> > +       struct net_device *peer;
> > +
> > +       if (new_hr < 0)
> > +               new_hr = 0;
> > +
> > +       rcu_read_lock();
> > +       peer = rcu_dereference(priv->peer);
> > +       if (unlikely(!peer))
> > +               goto out;
> > +
> > +       peer_priv = netdev_priv(peer);
> > +       priv->requested_headroom = new_hr;
> > +       new_hr = max(priv->requested_headroom, peer_priv->requested_headroom);
> > +       dev->needed_headroom = new_hr;
> > +       peer->needed_headroom = new_hr;
> > +
> > +out:
> > +       rcu_read_unlock();
> > +}
> > +
> I am not sure why new priv->requested_headroom is introduced. I think
> you can just compare new_hr with dev->needed_headroom and set the max
> value to both devices.

The requested_headroom headroom was intended to cope with with this
scenario:

brctl addif br0 veth0
brctl delif br0 veth0
brctl addif br1 veth0

If I set needed_headroom to the max value to both veth peers, after the
last call veth will use max(br0 headroom, br1 headroom), instead of br1
headroom.

Paolo