netdev - RE: [PATCH net-next] vxlan: distribute vxlan tunneled traffic across multiple TXQs

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:	Tue, 24 Dec 2013 12:27:19 +0000
From:	Sathya Perla <Sathya.Perla@...lex.Com>
To:	Stephen Hemminger <stephen@...workplumber.org>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"edumazet@...gle.com" <edumazet@...gle.com>
Subject: RE: [PATCH net-next] vxlan: distribute vxlan tunneled traffic
 across multiple TXQs

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@...workplumber.org]
> 
> On Mon, 23 Dec 2013 18:26:53 +0530
> Sathya Perla <sathya.perla@...lex.com> wrote:
> 
> > The vxlan driver sets itself as the socket owner for all the TX flows
> > it encapsulates (using vxlan_set_owner()) and assigns it's own skb
> > destructor. This causes all tunneled traffic to land up on only one TXQ
> > as all encapsulated skbs refer to the vxlan socket and not the original
> > socket.  Also, the vxlan skb destructor breaks some functionality for
> > tunneled traffic like wmem accounting and as Eric D. mentioned, TCP
> > small queues and FQ/pacing packet scheduler too.
> >
> > This patch removes vxlan ownership on tunneled skbs. This causes
> > tunneled traffic to be hashed into multiple TXQs based on the original
> > socket hash.
> >
...
> > -
> >  /* Compute source port for outgoing packet
> >   *   first choice to use L4 flow hash since it will spread
> >   *     better and maybe available from hardware
> > @@ -1514,8 +1500,6 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
> >  	ip6h->daddr	  = *daddr;
> >  	ip6h->saddr	  = *saddr;
> >
> > -	vxlan_set_owner(vs->sock->sk, skb);
> > -
> >  	err = handle_offloads(skb);
> >  	if (err)
> >  		return err;
> > @@ -1572,8 +1556,6 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
> >  	uh->len = htons(skb->len);
> >  	uh->check = 0;
> >
> > -	vxlan_set_owner(vs->sock->sk, skb);
> > -
> >  	err = handle_offloads(skb);
> >  	if (err)
> >  		return err;
> > @@ -1836,8 +1818,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct
> net_device *dev)
> >  		struct sk_buff *skb1;
> >
> >  		skb1 = skb_clone(skb, GFP_ATOMIC);
> > -		if (skb1)
> > +		if (skb1) {
> > +			skb1->sk = skb->sk;
> >  			vxlan_xmit_one(skb1, dev, rdst, did_rsc);
> > +		}
> >  	}
> >
> >  	dev_kfree_skb(skb);
> 
> The idea is good, but without the destructor there is nothing to keep
> the UDP socket from being destroyed while packet is being sent on another
> CPU.

Stephen, what is the consequence of freeing the UDP socket while a tunneled
skb is still in flight on another CPU? Wouldn't it be safe if the skb did not refer to the
vxlan socket at all?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html