Netdev List
 help / color / mirror / Atom feed
* Re: Multicast routing stops functioning after 4G multicast packets recived.
From: Hannes Frederic Sowa @ 2014-01-07 20:20 UTC (permalink / raw)
  To: Bob Falken, Julian Anastasov, netdev
In-Reply-To: <20140107201147.GG30393@order.stressinduktion.org>

On Tue, Jan 07, 2014 at 09:11:47PM +0100, Hannes Frederic Sowa wrote:
> On Tue, Jan 07, 2014 at 06:43:22PM +0100, Hannes Frederic Sowa wrote:
> > On Tue, Jan 07, 2014 at 06:01:44PM +0100, Bob Falken wrote:
> > > Hello,
> > > 
> > > I patched, kernel 3.2.53 yesterday,
> > > 8774002632packets, and ~9.1TB later, the multicast routing seems to function properly. :)
> > > 
> > > Kudos for fixing this.
> > > 
> > > I will keep checking the next days.
> > > 
> > > As for IPv6 MR, my current setup i.e: the Multicast source, does not support IPv6, so cannot do a check for that natively.
> > > 
> > > Unless I can translate IPv4 multicast into IPv6 multicast easily, using some iptable prerouting rules(?).
> > 
> > Thank you for testing!
> > 
> > I'll review the RCU regions again and will prepare the patches (before
> > introduction of ebc0ffae5dfb44 ("fib: RCU conversion of fib_lookup()")
> > and after.
> 
> It seems ip(6)mr_fib_lookup is not always called  from rcu section
> (ndo_start_xmit), so I had to restructure a bit. Could you retest this
> patch as preparation for a submission to stable? Thanks!
> 
> RCU conversion can be done later then.

Broken patch, sorry. Please try this one:

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 421a249..e5e9071 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -157,9 +157,12 @@ static struct mr_table *ipmr_get_table(struct net *net, u32 id)
 static int ipmr_fib_lookup(struct net *net, struct flowi4 *flp4,
 			   struct mr_table **mrt)
 {
-	struct ipmr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ipmr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
 			       flowi4_to_flowi(flp4), 0, &arg);
@@ -448,16 +451,22 @@ failure:
 
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	int err;
+	struct ipmr_result res;
 	struct net *net = dev_net(dev);
-	struct mr_table *mrt;
+
+	struct fib_lookup_arg arg = {
+		.result = &res,
+	};
+
 	struct flowi4 fl4 = {
 		.flowi4_oif	= dev->ifindex,
 		.flowi4_iif	= skb->skb_iif,
 		.flowi4_mark	= skb->mark,
 	};
-	int err;
 
-	err = ipmr_fib_lookup(net, &fl4, &mrt);
+	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
+			       flowi4_to_flowi(&fl4), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;
@@ -466,9 +475,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ipmr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, IGMPMSG_WHOLEPKT);
+	ipmr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			  IGMPMSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }
 
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index f365310..45ec621 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -141,9 +141,12 @@ static struct mr6_table *ip6mr_get_table(struct net *net, u32 id)
 static int ip6mr_fib_lookup(struct net *net, struct flowi6 *flp6,
 			    struct mr6_table **mrt)
 {
-	struct ip6mr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ip6mr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
 			       flowi6_to_flowi(flp6), 0, &arg);
@@ -693,16 +696,20 @@ static const struct inet6_protocol pim6_protocol = {
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 				      struct net_device *dev)
 {
+	int err;
+	struct ip6mr_result res;
 	struct net *net = dev_net(dev);
-	struct mr6_table *mrt;
 	struct flowi6 fl6 = {
 		.flowi6_oif	= dev->ifindex,
 		.flowi6_iif	= skb->skb_iif,
 		.flowi6_mark	= skb->mark,
 	};
-	int err;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+	};
 
-	err = ip6mr_fib_lookup(net, &fl6, &mrt);
+	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
+			flowi6_to_flowi(&fl6), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;
@@ -711,9 +718,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ip6mr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, MRT6MSG_WHOLEPKT);
+	ip6mr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			   MRT6MSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }
 

^ permalink raw reply related

* Re: [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols
From: Or Gerlitz @ 2014-01-07 20:19 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	netdev@vger.kernel.org, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <1389112433.26646.28.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Jan 7, 2014 at 6:33 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:
>>
>> +
>> +#define MAX_UDP_PORT (1 << 16)
>> +extern const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT];
>
> Thats 512 KB of memory.
> This will greatly impact forwarding performance of UDP packets with
> random ports, and will increase kernel memory size for embedded devices.

Re forwarding, are you referring to the case where the forwarded
packets are encapsulated? packets which are not encapusalted will be
flushed in the gro receive handler (this went out by mistake in V2 but
exists in V1)  if skb->encapsulation isn't set.

As for encapsulated packets, when you say random ports, are you
referring to a router which has multiple udp encapsulating protocols
where each uses different udp port? for this case and also to reduce
the memory footprint, we can use lookup in a list as done for the L2
protocols gro handlers in the list_for_each loop of dev_gro_receive(),
makes sense?

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-07 20:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Tom Herbert, Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	Linux Netdev List, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <1389124947.26646.54.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Jan 7, 2014 at 10:02 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-01-07 at 21:43 +0200, Or Gerlitz wrote:
>> On Tue, Jan 7, 2014 at 8:08 PM, Tom Herbert <therbert@google.com> wrote:
>
>> > Why ^ instead of != ?
>>
>> The XOR approach is very popular in the GRO stack, e.g see the IPv4 chain
>> of inet_gro_receive() && tcp_gro_receive(), I guess this might relates
>> to more efficient assembly code for ^ vs. != and/or the fast/elegant
>> transitive nature of that operator
>
> This trick is only needed/used when many compares are folded into a
> single conditional :
>
> if (a->f1 != b->f1 || a->f2 != b->f2)
>
> ->
>
> if (((a->f1 ^ b->f1) | (a->f2 ^ b->f2)) != 0)
>
> Please do not use XOR for a single compare.

OK, but just out of curiosity -- what's the reasoning? clarity or
efficiency or both?
>
>
>

^ permalink raw reply

* Re: Multicast routing stops functioning after 4G multicast packets recived.
From: Hannes Frederic Sowa @ 2014-01-07 20:11 UTC (permalink / raw)
  To: Bob Falken, Julian Anastasov, netdev
In-Reply-To: <20140107174322.GC30393@order.stressinduktion.org>

On Tue, Jan 07, 2014 at 06:43:22PM +0100, Hannes Frederic Sowa wrote:
> On Tue, Jan 07, 2014 at 06:01:44PM +0100, Bob Falken wrote:
> > Hello,
> > 
> > I patched, kernel 3.2.53 yesterday,
> > 8774002632packets, and ~9.1TB later, the multicast routing seems to function properly. :)
> > 
> > Kudos for fixing this.
> > 
> > I will keep checking the next days.
> > 
> > As for IPv6 MR, my current setup i.e: the Multicast source, does not support IPv6, so cannot do a check for that natively.
> > 
> > Unless I can translate IPv4 multicast into IPv6 multicast easily, using some iptable prerouting rules(?).
> 
> Thank you for testing!
> 
> I'll review the RCU regions again and will prepare the patches (before
> introduction of ebc0ffae5dfb44 ("fib: RCU conversion of fib_lookup()")
> and after.

It seems ip(6)mr_fib_lookup is not always called  from rcu section
(ndo_start_xmit), so I had to restructure a bit. Could you retest this
patch as preparation for a submission to stable? Thanks!

RCU conversion can be done later then.

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 421a249..9ae4ae7 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -157,9 +157,12 @@ static struct mr_table *ipmr_get_table(struct net *net, u32 id)
 static int ipmr_fib_lookup(struct net *net, struct flowi4 *flp4,
 			   struct mr_table **mrt)
 {
-	struct ipmr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ipmr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
 			       flowi4_to_flowi(flp4), 0, &arg);
@@ -448,16 +451,23 @@ failure:
 
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	int err;
+	struct ipmr_result res;
 	struct net *net = dev_net(dev);
-	struct mr_table *mrt;
+
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
+
 	struct flowi4 fl4 = {
 		.flowi4_oif	= dev->ifindex,
 		.flowi4_iif	= skb->skb_iif,
 		.flowi4_mark	= skb->mark,
 	};
-	int err;
 
-	err = ipmr_fib_lookup(net, &fl4, &mrt);
+	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
+			       flowi4_to_flowi(&fl4), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;
@@ -466,9 +476,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ipmr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, IGMPMSG_WHOLEPKT);
+	ipmr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			  IGMPMSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }
 
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index f365310..45ec621 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -141,9 +141,12 @@ static struct mr6_table *ip6mr_get_table(struct net *net, u32 id)
 static int ip6mr_fib_lookup(struct net *net, struct flowi6 *flp6,
 			    struct mr6_table **mrt)
 {
-	struct ip6mr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ip6mr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
 			       flowi6_to_flowi(flp6), 0, &arg);
@@ -693,16 +696,20 @@ static const struct inet6_protocol pim6_protocol = {
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 				      struct net_device *dev)
 {
+	int err;
+	struct ip6mr_result res;
 	struct net *net = dev_net(dev);
-	struct mr6_table *mrt;
 	struct flowi6 fl6 = {
 		.flowi6_oif	= dev->ifindex,
 		.flowi6_iif	= skb->skb_iif,
 		.flowi6_mark	= skb->mark,
 	};
-	int err;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+	};
 
-	err = ip6mr_fib_lookup(net, &fl6, &mrt);
+	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
+			flowi6_to_flowi(&fl6), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;
@@ -711,9 +718,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ip6mr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, MRT6MSG_WHOLEPKT);
+	ip6mr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			   MRT6MSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }
 

^ permalink raw reply related

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-07 20:10 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	netdev@vger.kernel.org, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <1389125042.26646.56.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Jan 7, 2014 at 10:04 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-01-07 at 21:43 +0200, Or Gerlitz wrote:
>> On Tue, Jan 7, 2014 at 6:34 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:
>> >> Add gro handlers for vxlan using the udp gro infrastructure
>> >>
>> >
>> >
>> >>  static void vxlan_notify_add_rx_port(struct sock *sk)
>> >>  {
>> >> @@ -568,6 +661,8 @@ static void vxlan_notify_add_rx_port(struct sock *sk)
>> >>                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>> >>                                                           port);
>> >>       }
>> >> +     if (sa_family == AF_INET)
>> >> +             udp_add_offload(&vxlan_offload, port);
>> >>       rcu_read_unlock();
>> >>  }
>> >>
>> >
>> > This means two vxlan tunnels can not share same port.
>> > Is that a valid assertion ?
>>
>> nope -- the vxlan driver opens a listener udp socket per per listening
>> port, but N > 1 vxlan tunnels can sit on that port, the driver does
>> further demuxing based on the vnid carried in the vxlan header, see
>> the call to vxlan_find_vni() from vxlan_rcv()
>> --
>
> So if we use same port, it seems to me udp_del_offload() of the first
> dismantled tunnel will remove the offload for the remaining tunnel.
>
> You forgot to implement a refcount somehow.

no, the vxlan driver does it for me on a higher level, you can see in
vxlan_sock_release() that vxlan_notify_del_rx_port() is called only if
the ref count which is associated with the listener socket drops to
zero.

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Eric Dumazet @ 2014-01-07 20:04 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	netdev@vger.kernel.org, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <CAJZOPZ+8bNi+HZ=eyGBwzFDdrkroMVyPzoGERFqcnZPq3rBZ6Q@mail.gmail.com>

On Tue, 2014-01-07 at 21:43 +0200, Or Gerlitz wrote:
> On Tue, Jan 7, 2014 at 6:34 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:
> >> Add gro handlers for vxlan using the udp gro infrastructure
> >>
> >
> >
> >>  static void vxlan_notify_add_rx_port(struct sock *sk)
> >>  {
> >> @@ -568,6 +661,8 @@ static void vxlan_notify_add_rx_port(struct sock *sk)
> >>                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
> >>                                                           port);
> >>       }
> >> +     if (sa_family == AF_INET)
> >> +             udp_add_offload(&vxlan_offload, port);
> >>       rcu_read_unlock();
> >>  }
> >>
> >
> > This means two vxlan tunnels can not share same port.
> > Is that a valid assertion ?
> 
> nope -- the vxlan driver opens a listener udp socket per per listening
> port, but N > 1 vxlan tunnels can sit on that port, the driver does
> further demuxing based on the vnid carried in the vxlan header, see
> the call to vxlan_find_vni() from vxlan_rcv()
> --

So if we use same port, it seems to me udp_del_offload() of the first
dismantled tunnel will remove the offload for the remaining tunnel.

You forgot to implement a refcount somehow.

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Eric Dumazet @ 2014-01-07 20:02 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Tom Herbert, Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	Linux Netdev List, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <CAJZOPZKHC1t6tGjZ9ZyTD912-5Bw_Vtin8N2MnqkOi4W2wPrzA@mail.gmail.com>

On Tue, 2014-01-07 at 21:43 +0200, Or Gerlitz wrote:
> On Tue, Jan 7, 2014 at 8:08 PM, Tom Herbert <therbert@google.com> wrote:

> > Why ^ instead of != ?
> 
> The XOR approach is very popular in the GRO stack, e.g see the IPv4 chain
> of inet_gro_receive() && tcp_gro_receive(), I guess this might relates
> to more efficient assembly code for ^ vs. != and/or the fast/elegant
> transitive nature of that operator

This trick is only needed/used when many compares are folded into a
single conditional :

if (a->f1 != b->f1 || a->f2 != b->f2)

->

if (((a->f1 ^ b->f1) | (a->f2 ^ b->f2)) != 0)

Please do not use XOR for a single compare.

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Tom Herbert @ 2014-01-07 19:52 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	Linux Netdev List, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <CAJZOPZLsMvmHwmMjhsuKb__2HncMXMm=p6UFnT4XX5d8hZnGxw@mail.gmail.com>

On Tue, Jan 7, 2014 at 11:37 AM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
> On Tue, Jan 7, 2014 at 8:08 PM, Tom Herbert <therbert@google.com> wrote:
>>
>> On Tue, Jan 7, 2014 at 7:29 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> > +static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct
>> > sk_buff *skb)
>> > +{
>> > +       struct sk_buff *p, **pp = NULL;
>> > +       struct vxlanhdr *vh, *vh2;
>> > +       struct ethhdr *eh;
>> > +       unsigned int hlen, off, off_eth;
>> > +       const struct packet_offload *ptype;
>> > +       __be16 type;
>> > +       int flush = 1;
>> > +
>> > +       off  = skb_gro_offset(skb);
>> > +       hlen = off + sizeof(*vh);
>> > +       vh   = skb_gro_header_fast(skb, off);
>> > +       if (skb_gro_header_hard(skb, hlen)) {
>> > +               vh = skb_gro_header_slow(skb, hlen, off);
>> > +               if (unlikely(!vh))
>> > +                       goto out;
>> > +       }
>> > +
>> > +       flush = 0;
>> > +
>> > +       for (p = *head; p; p = p->next) {
>> > +               if (!NAPI_GRO_CB(p)->same_flow)
>> > +                       continue;
>> > +
>> > +               vh2 = (struct vxlanhdr   *)(p->data + off);
>> > +               if (vh->vx_vni ^ vh2->vx_vni) {
>>
>> Why ^ instead of != ?
>
>
> The XOR approach is very popular in the GRO stack, e.g see the IPv4 chain of
> inet_gro_receive() && tcp_gro_receive(), I guess this might relates to more
> efficient assembly code for ^ vs. != and/or the fast/elegant transitive
> nature of that operator
>
These arguments may have applied twenty years ago, but I seriously
doubt this is going to outwit any modern compiler into producing more
efficient code. To me, it's just making code less readable.

>>
>> > +                       NAPI_GRO_CB(p)->same_flow = 0;
>> > +                       continue;
>> > +               }
>> > +               goto found;
>> > +       }
>
>

^ permalink raw reply

* Re: [PATCH RFC v2 0/13] vti4: prepare namespace and interfamily support.
From: Christophe Gouault @ 2014-01-07 19:45 UTC (permalink / raw)
  To: Steffen Klassert, netdev; +Cc: Saurabh Mohan
In-Reply-To: <52CC2714.5080600@6wind.com>

Le 07/01/2014 17:11, Christophe Gouault a écrit :
> On 12/16/2013 10:18 AM, Steffen Klassert wrote:
>> This patchset prepares vti4 for proper namespace and interfamily support.
>>
>> Currently the receive hook is in the middle of the decapsulation
>> process, some of the header pointers point still into the IPsec packet
>> others point already into the decapsulated packet. This makes it
>> very unflexible and proper namespace and interfamily support can't
>> be done as it is.
>>
>> The patchset that implements an IPsec protocol multiplexer, so that vti
>> can register it's own receive path hooks. Further it makes the i_key
>> usable for vti and changes the vti4 code to do the following:
>>
>> vti uses the IPsec protocol multiplexer to register it's
>> own receive side hooks for ESP, AH and IPCOMP.
>>
>> Vti does the following on receive side:
>>
>> 1. Do an input policy check for the IPsec packet we received.
>>     This is required because this packet could be already
>>     processed by IPsec (tunnel in tunnel or a block policy
>>     is present), so an inbound policy check is needed.
>>
>> 2. Clean the skb to not leak informations on namespace
>>     transitions.
>>
>> 3. Mark the packet with the i_key. The policy and the state
>>     must match this key now. Policy and state belong to the vti
>>     namespace and policy enforcement is done at the further layers.
>>
>> 4. Call the generic xfrm layer to do decryption and decapsulation.
>>
>> 5. Wait for a callback from the xfrm layer to properly update
>>     the device statistics.
>
> Sorry for my late comments, I had to delay my tests due to Christmas and
> New Year's celebrations.
>
> I have a few comments about your proposed patches:
>
> In input, the vti tunnel processing does not follow the usual tunnel
> processing. Conventionally, the packets are first decapsulated, then
> only the skbuff interface is changed to the tunnel interface. In the vti
> code, the interface is changed before IPsec decryption, hence before
> decapsulation.
>
> It results in a configuration asymmetry when we later support cross
> netns: the outer SAs and SPs must be defined in the outer netns, while
> the inner SAs and SPs must be defined in the inner netns. This is a
> little disturbing.
>
>> On transmit side:
>>
>> 1. Mark the packet with the o_key. The policy and the state
>>     must match this key now.
>>
>> 2. Do a xfrm_lookup on the original packet with the mark applied.
>>
>> 3. Check if we got an IPsec route.
>>
>> 4. Clean the skb to not leak informations on namespace
>>     transitions.
>>
>> 5. Attach the dst_enty we got from the xfrm_lookup to the skb.
>>
>> 6. Call dst_output to do the IPsec processing.
>>
>> 7. Do the device statistics.
>
> In output, when the route points to a vti interface, the global SPD
> lookup is not bypassed: an SPD lookup is still performed for a global
> SPD (i.e. without applying the vti mark). Then only the packet can enter
> the vti interface, in which a second SPD lookup is done, in the vti
> interface SPD (i.e. after applying the vti mark). Of course if the
> global SPD lookup returned a tunnel mode policy, then the packet may
> finally not enter the vti interface, because a new route is looked up
> after the IPsec encapsulation.
>
> My understanding of the vti interface interest is enabling to use
> routing (possibly dynamic routes) *instead* of complex security
> policies. And in this use case I expect that entering a vti interface
> will *override* the global policies (in the same manner as socket
> policies override the global policies).
>
> Otherwise, if we want to mix global and vti policies on the same
> machine, then we must carefully define global policies that do not match
> traffic destined to vti interfaces.
>
> Note that setting the NOXFRM flag on the vti interface does not work
> around this issue, it disables both the global and vti SPD lookup and
> the traffic is finally dropped.

I finally found a way of bypassing the global SPD lookup, by explicitly 
adding a global policy of higher priority than other global policies:

# bypass outbound SPD lookup if packet is destined to vti1
ip xfrm policy add dir out dev vti1 mark 0 priority 0 allow

>> Changes from v1:
>>
>> - Rebased to current net-next.
>> - Fix a rcu lockdep complaint in xfrm protocol
>> registration/deregistration.
>> - Fix usage of a ipv4 specific callback handler in generic code.
>> - Use skb_scrub_packet() to clear the skb in vti_rcv(), suggested by
>>    Nicolas Dichtel.
>> - Add support for IPCOMP.
>> - Support inter address family tunneling.
>>
>> I'd take this into the ipsec-next tree after some testing if noone
>> has further suggestions or objections.
>>
>> I have the ipv6 side ready too, this will be a separate patchset.
>> The ipv6 patchset has dependencies against the ipv4 patchset, so I
>> hold it back until we have got the ipv4 side merged.
>>

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-07 19:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	netdev@vger.kernel.org, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <1389112461.26646.29.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Jan 7, 2014 at 6:34 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:
>> Add gro handlers for vxlan using the udp gro infrastructure
>>
>
>
>>  static void vxlan_notify_add_rx_port(struct sock *sk)
>>  {
>> @@ -568,6 +661,8 @@ static void vxlan_notify_add_rx_port(struct sock *sk)
>>                       dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>>                                                           port);
>>       }
>> +     if (sa_family == AF_INET)
>> +             udp_add_offload(&vxlan_offload, port);
>>       rcu_read_unlock();
>>  }
>>
>
> This means two vxlan tunnels can not share same port.
> Is that a valid assertion ?

nope -- the vxlan driver opens a listener udp socket per per listening
port, but N > 1 vxlan tunnels can sit on that port, the driver does
further demuxing based on the vnid carried in the vxlan header, see
the call to vxlan_find_vni() from vxlan_rcv()

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-07 19:43 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	Linux Netdev List, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <CA+mtBx-fUe_VbUUnjEVXeJO=87yypoDuMfoctO0Q3nMx+6UZFA@mail.gmail.com>

On Tue, Jan 7, 2014 at 8:08 PM, Tom Herbert <therbert@google.com> wrote:
> On Tue, Jan 7, 2014 at 7:29 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> +static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff *skb)
>> +{
>> +       struct sk_buff *p, **pp = NULL;
>> +       struct vxlanhdr *vh, *vh2;
>> +       struct ethhdr *eh;
>> +       unsigned int hlen, off, off_eth;
>> +       const struct packet_offload *ptype;
>> +       __be16 type;
>> +       int flush = 1;
>> +
>> +       off  = skb_gro_offset(skb);
>> +       hlen = off + sizeof(*vh);
>> +       vh   = skb_gro_header_fast(skb, off);
>> +       if (skb_gro_header_hard(skb, hlen)) {
>> +               vh = skb_gro_header_slow(skb, hlen, off);
>> +               if (unlikely(!vh))
>> +                       goto out;
>> +       }
>> +
>> +       flush = 0;
>> +
>> +       for (p = *head; p; p = p->next) {
>> +               if (!NAPI_GRO_CB(p)->same_flow)
>> +                       continue;
>> +
>> +               vh2 = (struct vxlanhdr   *)(p->data + off);
>> +               if (vh->vx_vni ^ vh2->vx_vni) {
>
> Why ^ instead of != ?

The XOR approach is very popular in the GRO stack, e.g see the IPv4 chain
of inet_gro_receive() && tcp_gro_receive(), I guess this might relates
to more efficient assembly code for ^ vs. != and/or the fast/elegant
transitive nature of that operator

^ permalink raw reply

* Re: [PATCH] ipv6: pcpu_tstats.syncp should be initialised in ip6_vti.c
From: David Miller @ 2014-01-07 19:15 UTC (permalink / raw)
  To: roy.qing.li; +Cc: netdev, fengguang.wu
In-Reply-To: <1389080383-25655-1-git-send-email-roy.qing.li@gmail.com>

From: roy.qing.li@gmail.com
Date: Tue,  7 Jan 2014 15:39:43 +0800

> From: Li RongQing <roy.qing.li@gmail.com>
> 
> initialise pcpu_tstats.syncp to kill the calltrace
 ...
> Before 469bdcefdc ("ipv6: fix the use of pcpu_tstats in ip6_vti.c"),
> the pcpu_tstats.syncp is not used to pretect the 64bit elements of
> pcpu_tstats, so not appear this calltrace.
> 
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>

Applied, thanks.

It looks like ip6_tunnel.c and sit.c are OK in this area.

^ permalink raw reply

* Re: [PATCH -next] qlcnic: use vzalloc() instead of vmalloc()/memset(0)
From: Joe Perches @ 2014-01-07 19:14 UTC (permalink / raw)
  To: Jitendra Kalsaria
  Cc: Wei Yongjun, Himanshu Madhani, Rajesh Borundia, Shahed Shaikh,
	Sony Chacko, Sucheta Chakraborty, yongjun_wei@trendmicro.com.cn,
	Dept-Eng Linux Driver, netdev
In-Reply-To: <BECD8E8A1B550B48A1BF97C13991F60E46CF1A67@avmb2.qlogic.org>

On Tue, 2014-01-07 at 18:22 +0000, Jitendra Kalsaria wrote:
> On 1/7/14 5:48 AM, "Wei Yongjun" <weiyj.lk@gmail.com> wrote:
> >From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >Use vzalloc() instead of vmalloc() and memset(0).
[]
> >diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c

> >@@ -935,11 +935,10 @@ static ssize_t qlcnic_sysfs_read_pci_config(struct
> >file *file,
> > 		return QL_STATUS_INVALID_PARAM;
> > 
> > 	pci_info_sz = pci_func_count * sizeof(*pci_info);
> >-	pci_info = vmalloc(pci_info_sz);
> >+	pci_info = vzalloc(pci_info_sz);

Maybe this is a bit more comprehensive?

pci_info_sz is now used once and might as well be eliminated.
Use ETH_ALEN for 6
Remove memset to 0 of buf, add set to 0 of only unset member.

---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h       |  2 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c | 11 ++++-------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 35d4876..8d7aa4c 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -1267,7 +1267,7 @@ struct qlcnic_pci_func_cfg {
 	u16	port_num;
 	u8	pci_func;
 	u8	func_state;
-	u8	def_mac_addr[6];
+	u8	def_mac_addr[ETH_ALEN];
 };
 
 struct qlcnic_npar_func_cfg {
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
index b529667..4ac8b20 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
@@ -927,31 +927,28 @@ static ssize_t qlcnic_sysfs_read_pci_config(struct file *file,
 	u32 pci_func_count = qlcnic_get_pci_func_count(adapter);
 	struct qlcnic_pci_func_cfg *pci_cfg;
 	struct qlcnic_pci_info *pci_info;
-	size_t pci_info_sz, pci_cfg_sz;
+	size_t pci_cfg_sz;
 	int i, ret;
 
 	pci_cfg_sz = pci_func_count * sizeof(*pci_cfg);
 	if (size != pci_cfg_sz)
 		return QL_STATUS_INVALID_PARAM;
 
-	pci_info_sz = pci_func_count * sizeof(*pci_info);
-	pci_info = vmalloc(pci_info_sz);
+	pci_info = vzalloc(pci_func_count * sizeof(*pci_info));
 	if (!pci_info)
 		return -ENOMEM;
 
-	memset(pci_info, 0, pci_info_sz);
-	memset(buf, 0, pci_cfg_sz);
-	pci_cfg = (struct qlcnic_pci_func_cfg *)buf;
-
 	ret = qlcnic_get_pci_info(adapter, pci_info);
 	if (ret) {
 		vfree(pci_info);
 		return ret;
 	}
 
+	pci_cfg = (struct qlcnic_pci_func_cfg *)buf;
 	for (i = 0; i < pci_func_count; i++) {
 		pci_cfg[i].pci_func = pci_info[i].id;
 		pci_cfg[i].func_type = pci_info[i].type;
+		pci_cfg[i].func_state = 0;
 		pci_cfg[i].port_num = pci_info[i].default_port;
 		pci_cfg[i].min_bw = pci_info[i].tx_min_bw;
 		pci_cfg[i].max_bw = pci_info[i].tx_max_bw;

^ permalink raw reply related

* Re: [PATCH net-next v5] IPv6: add the option to use anycast addresses as source addresses in echo reply
From: Hannes Frederic Sowa @ 2014-01-07 19:14 UTC (permalink / raw)
  To: Francois-Xavier Le Bail
  Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki Yoshifuji, Patrick McHardy
In-Reply-To: <1389103047-3380-1-git-send-email-fx.lebail@yahoo.com>

On Tue, Jan 07, 2014 at 02:57:27PM +0100, Francois-Xavier Le Bail wrote:
> This change allows to follow a recommandation of RFC4942.
> 
> - Add "anycast_src_echo_reply" sysctl to control the use of anycast addresses
>   as source addresses for ICMPv6 echo reply. This sysctl is false by default
>   to preserve existing behavior.
> - Add inline check ipv6_anycast_destination().
> - Use them in icmpv6_echo_reply().
> 
> Reference:
> RFC4942 - IPv6 Transition/Coexistence Security Considerations
>    (http://tools.ietf.org/html/rfc4942#section-2.1.6)
> 
> 2.1.6. Anycast Traffic Identification and Security
> 
>    [...]
>    To avoid exposing knowledge about the internal structure of the
>    network, it is recommended that anycast servers now take advantage of
>    the ability to return responses with the anycast address as the
>    source address if possible.
> 
> Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>

Works and best solution IMHO.

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks,

  Hannes

^ permalink raw reply

* Re: [PATCH v3 net-next 1/3] sh_eth: Add support for r7s72100
From: David Miller @ 2014-01-07 19:07 UTC (permalink / raw)
  To: horms+renesas
  Cc: netdev, linux-sh, linux-arm-kernel, magnus.damm, sergei.shtylyov
In-Reply-To: <1389061635-4083-2-git-send-email-horms+renesas@verge.net.au>

From: Simon Horman <horms+renesas@verge.net.au>
Date: Tue,  7 Jan 2014 11:27:13 +0900

> @@ -318,6 +371,14 @@ static int sh_eth_is_gether(struct sh_eth_private *mdp)
>  		return 0;
>  }
>  
> +static int sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> +{
> +	if (mdp->reg_offset == sh_eth_offset_fast_rz)
> +		return 1;
> +	else
> +		return 0;
> +}

Please make this return a boolean and use true/false.

> @@ -2061,6 +2155,10 @@ static struct net_device_stats *sh_eth_get_stats(struct net_device *ndev)
>  {
>  	struct sh_eth_private *mdp = netdev_priv(ndev);
>  
> +	if (sh_eth_is_rz_fast_ether(mdp)) {
> +		return &ndev->stats;
> +	}

Single statement basic blocks do not need curly braces, therefore
please remove them here.

THanks.

^ permalink raw reply

* Re: Use of 'SIOCDEVPRIVATE' in ethernet drivers.
From: David Miller @ 2014-01-07 19:05 UTC (permalink / raw)
  To: giri.reddy; +Cc: netdev
In-Reply-To: <124064C45E1FC84193A69A61ED3A1F46E22D07@AVMB1.qlogic.org>

From: Giri Reddy <giri.reddy@qlogic.com>
Date: Tue, 7 Jan 2014 18:21:17 +0000

> Are we still allowed to use 'SIOCDEVPRIVATE' in drivers if there the
> deprecation efforts are on hold.

To say that use of SIOCDEVPRIVATE is discouraged would be an
understatement.

Please create a generic ethtool based facility that other drivers in
similar situations can use as well.

^ permalink raw reply

* Re: [PATCH v2 2/2] ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE
From: Hannes Frederic Sowa @ 2014-01-07 19:01 UTC (permalink / raw)
  To: Thomas Haller; +Cc: Jiri Pirko, netdev, stephen, dcbw
In-Reply-To: <1389119577.2248.16.camel@weing>

Hi,

On Tue, Jan 07, 2014 at 07:32:57PM +0100, Thomas Haller wrote:
> On Tue, 2014-01-07 at 17:28 +0100, Hannes Frederic Sowa wrote:
> > On Tue, Jan 07, 2014 at 03:39:13PM +0100, Thomas Haller wrote:
> > > Also, when adding the NOPREFIXROUTE flag to an already existing address,
> > > check if there there is a prefix that was likly added by the kernel
> > > and delete it.
> > 
> > Hmm, could you give a bit more details why you have done this? I find
> > that a bit counterintuitive. Maybe it has a reason?
> > 
> 
> You find the behavior or the commit message counterintuitive? Didn't you
> suggest this behavior in your email from "7 Jan 2014 13:01:11 +0100"?

I guess I was a bit confused, sorry. I think I confused the deleted and modify
case. However:

So we have the following changes on addresses:

add is simple: just as in the first patch

modify: is a bit hairy. To be extremly exact, we would have to recreate the
	route with proper metrics etc. so delete in any case and reinsert.
	I really dislike removing a route someone else might have inserted
	manually, and this is a likely scenario.

	Somehow I tend to just don't allow NOPREFIXROUTE on modify at all and
	just return a proper error value. What do you think? What would be the
	best behavior for NM?

delete: if IFA_F_NOPREFIXROUTE is set, we don't care about removing a prefix
	route, it must be set by user space and should get cleaned up by user
	space

> 
> 
> For v3 I will reword the commit message. How about the following:
> 
>     ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
>     
>     Refactor the deletion/update of prefix routes when removing an
>     address. Now, consider IFA_F_NOPREFIXROUTE and if there is an address
>     present with this flag, to not cleanup the route. Instead, assume
>     that userspace is taking care of this prefix.
>     
>     Also perform the same cleanup, when userspace changes an existing address
>     to add NOPREFIXROUTE to an address that didn't have this flag. We do this
>     because when the address was added, a prefix route was created for it.
>     Since the user now wants to handle this route by himself, we remove it again.
>     
>     As before, a prefix route only gets removed, if there is no address
>     that might need it. Or, if there are only non-permanent addresses,
>     update the lifetime of the route.

If we want go with the current modify behavior this sounds good.

Thanks,

  Hannes

^ permalink raw reply

* Re: [PATCH] mlx4_en: Select PTP_1588_CLOCK
From: Shawn Bohrer @ 2014-01-07 18:57 UTC (permalink / raw)
  To: David S. Miller
  Cc: Or Gerlitz, Amir Vadai, Richard Cochran, netdev, tomk,
	Hadar Hen Zion, Shawn Bohrer
In-Reply-To: <1389120557-6773-1-git-send-email-shawn.bohrer@gmail.com>

On Tue, Jan 07, 2014 at 12:49:17PM -0600, Shawn Bohrer wrote:
> From: Shawn Bohrer <sbohrer@rgmadvisors.com>
> 
> Now that mlx4_en includes a PHC driver it must select PTP_1588_CLOCK.
> 
>    drivers/built-in.o: In function `mlx4_en_get_ts_info':
> >> en_ethtool.c:(.text+0x391a11): undefined reference to `ptp_clock_index'
>    drivers/built-in.o: In function `mlx4_en_remove_timestamp':
> >> (.text+0x397913): undefined reference to `ptp_clock_unregister'
>    drivers/built-in.o: In function `mlx4_en_init_timestamp':
> >> (.text+0x397b20): undefined reference to `ptp_clock_register'
> 
> Fixes: ad7d4eaed995d ("mlx4_en: Add PTP hardware clock")
> Signed-off-by: Shawn Bohrer <sbohrer@rgmadvisors.com>

Sorry forgot to add this is against net-next.

--
Shawn

^ permalink raw reply

* [PATCH] mlx4_en: Select PTP_1588_CLOCK
From: Shawn Bohrer @ 2014-01-07 18:49 UTC (permalink / raw)
  To: David S. Miller
  Cc: Or Gerlitz, Amir Vadai, Richard Cochran, netdev, tomk,
	Hadar Hen Zion, Shawn Bohrer

From: Shawn Bohrer <sbohrer@rgmadvisors.com>

Now that mlx4_en includes a PHC driver it must select PTP_1588_CLOCK.

   drivers/built-in.o: In function `mlx4_en_get_ts_info':
>> en_ethtool.c:(.text+0x391a11): undefined reference to `ptp_clock_index'
   drivers/built-in.o: In function `mlx4_en_remove_timestamp':
>> (.text+0x397913): undefined reference to `ptp_clock_unregister'
   drivers/built-in.o: In function `mlx4_en_init_timestamp':
>> (.text+0x397b20): undefined reference to `ptp_clock_register'

Fixes: ad7d4eaed995d ("mlx4_en: Add PTP hardware clock")
Signed-off-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
---
 drivers/net/ethernet/mellanox/mlx4/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index eb520ab..563495d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -6,6 +6,7 @@ config MLX4_EN
 	tristate "Mellanox Technologies 10Gbit Ethernet support"
 	depends on PCI
 	select MLX4_CORE
+	select PTP_1588_CLOCK
 	---help---
 	  This driver supports Mellanox Technologies ConnectX Ethernet
 	  devices.
-- 
1.7.7.6

^ permalink raw reply related

* Re: [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols
From: Tom Herbert @ 2014-01-07 18:44 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Jerry Chu, Eric Dumazet, Herbert Xu, Linux Netdev List,
	David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <1389108594-665-2-git-send-email-ogerlitz@mellanox.com>

Or, thanks for posting the patches!

We should also support the case where direct encapsulation is being
done, that is there is no encapsulation header after UDP and the
protocol of the encapsulated packet is inferred by the port number
(e.g. GRE/UDP, TCP/UDP, SCTP/UDP, etc.). This is probably an
additional field in net_offload struct for next protocol, a little
more API, and pretty trivial handlers in UDP code.


On Tue, Jan 7, 2014 at 7:29 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Add GRO handlers for protocols that do UDP encapsulation, with the intent of
> being able to coalesce packets which encapsulate packets belonging to
> the same TCP session.
>
> For GRO purposes, the destination UDP port takes the role of the ether type
> field in the ethernet header or the next protocol in the IP header.
>
> The UDP GRO handler will only attempt to coalesce packets whose destination
> port is registered to have gro handler.
>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
>  include/net/protocol.h |    6 ++++
>  net/ipv4/protocol.c    |   21 ++++++++++++++
>  net/ipv4/udp_offload.c |   69 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 96 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/protocol.h b/include/net/protocol.h
> index fbf7676..d776c08 100644
> --- a/include/net/protocol.h
> +++ b/include/net/protocol.h
> @@ -92,6 +92,10 @@ extern const struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS];
>  extern const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS];
>  extern const struct net_offload __rcu *inet6_offloads[MAX_INET_PROTOS];
>
> +
> +#define MAX_UDP_PORT (1 << 16)
> +extern const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT];
> +
>  #if IS_ENABLED(CONFIG_IPV6)
>  extern const struct inet6_protocol __rcu *inet6_protos[MAX_INET_PROTOS];
>  #endif
> @@ -102,6 +106,8 @@ int inet_add_offload(const struct net_offload *prot, unsigned char num);
>  int inet_del_offload(const struct net_offload *prot, unsigned char num);
>  void inet_register_protosw(struct inet_protosw *p);
>  void inet_unregister_protosw(struct inet_protosw *p);
> +int udp_add_offload(const struct net_offload *prot, __be16 port);
> +int udp_del_offload(const struct net_offload *prot, __be16 port);
>
>  #if IS_ENABLED(CONFIG_IPV6)
>  int inet6_add_protocol(const struct inet6_protocol *prot, unsigned char num);
> diff --git a/net/ipv4/protocol.c b/net/ipv4/protocol.c
> index 46d6a1c..426eae5 100644
> --- a/net/ipv4/protocol.c
> +++ b/net/ipv4/protocol.c
> @@ -30,6 +30,7 @@
>
>  const struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS] __read_mostly;
>  const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS] __read_mostly;
> +const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT] __read_mostly;
>
>  int inet_add_protocol(const struct net_protocol *prot, unsigned char protocol)
>  {
> @@ -51,6 +52,13 @@ int inet_add_offload(const struct net_offload *prot, unsigned char protocol)
>  }
>  EXPORT_SYMBOL(inet_add_offload);
>
> +int udp_add_offload(const struct net_offload *prot, __be16 port)
> +{
> +       return !cmpxchg((const struct net_offload **)&udp_offloads[port],
> +                       NULL, prot) ? 0 : -1;
> +}
> +EXPORT_SYMBOL(udp_add_offload);
> +
>  int inet_del_protocol(const struct net_protocol *prot, unsigned char protocol)
>  {
>         int ret;
> @@ -76,3 +84,16 @@ int inet_del_offload(const struct net_offload *prot, unsigned char protocol)
>         return ret;
>  }
>  EXPORT_SYMBOL(inet_del_offload);
> +
> +int udp_del_offload(const struct net_offload *prot, __be16 port)
> +{
> +       int ret;
> +
> +       ret = (cmpxchg((const struct net_offload **)&udp_offloads[port],
> +                      prot, NULL) == prot) ? 0 : -1;
> +
> +       synchronize_net();
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL(udp_del_offload);
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 79c62bd..0a8fdd6 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -89,10 +89,79 @@ out:
>         return segs;
>  }
>
> +
> +static struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb)
> +{
> +       const struct net_offload *ops;
> +       struct sk_buff *p, **pp = NULL;
> +       struct udphdr *uh, *uh2;
> +       unsigned int hlen, off;
> +       int flush = 1;
> +
> +       off  = skb_gro_offset(skb);
> +       hlen = off + sizeof(*uh);
> +       uh   = skb_gro_header_fast(skb, off);
> +       if (skb_gro_header_hard(skb, hlen)) {
> +               uh = skb_gro_header_slow(skb, hlen, off);
> +               if (unlikely(!uh))
> +                       goto out;
> +       }
> +
> +       rcu_read_lock();
> +       ops = rcu_dereference(udp_offloads[uh->dest]);
> +       if (!ops || !ops->callbacks.gro_receive)
> +               goto out_unlock;
> +
> +       flush = 0;
> +
> +       for (p = *head; p; p = p->next) {
> +               if (!NAPI_GRO_CB(p)->same_flow)
> +                       continue;
> +
> +               uh2 = (struct udphdr   *)(p->data + off);
> +               if ((*(u32 *)&uh->source ^ *(u32 *)&uh2->source)) {
> +                       NAPI_GRO_CB(p)->same_flow = 0;
> +                       continue;
> +               }
> +               goto found;
> +       }
> +
> +found:
> +       skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
> +       pp = ops->callbacks.gro_receive(head, skb);
> +
> +out_unlock:
> +       rcu_read_unlock();
> +out:
> +       NAPI_GRO_CB(skb)->flush |= flush;
> +
> +       return pp;
> +}
> +
> +static int udp_gro_complete(struct sk_buff *skb, int nhoff)
> +{
> +       const struct net_offload *ops;
> +       __be16 newlen = htons(skb->len - nhoff);
> +       struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
> +       int err = -ENOSYS;
> +
> +       uh->len = newlen;
> +
> +       rcu_read_lock();
> +       ops = rcu_dereference(udp_offloads[uh->dest]);
> +       if (ops && ops->callbacks.gro_complete)
> +               err = ops->callbacks.gro_complete(skb, nhoff + sizeof(struct udphdr));
> +
> +       rcu_read_unlock();
> +       return err;
> +}
> +
>  static const struct net_offload udpv4_offload = {
>         .callbacks = {
>                 .gso_send_check = udp4_ufo_send_check,
>                 .gso_segment = udp4_ufo_fragment,
> +               .gro_receive  = udp_gro_receive,
> +               .gro_complete = udp_gro_complete,
>         },
>  };
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 2/2] ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE
From: Thomas Haller @ 2014-01-07 18:32 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw
In-Reply-To: <20140107162847.GB30393@order.stressinduktion.org>

[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]

On Tue, 2014-01-07 at 17:28 +0100, Hannes Frederic Sowa wrote:
> On Tue, Jan 07, 2014 at 03:39:13PM +0100, Thomas Haller wrote:
> > Also, when adding the NOPREFIXROUTE flag to an already existing address,
> > check if there there is a prefix that was likly added by the kernel
> > and delete it.
> 
> Hmm, could you give a bit more details why you have done this? I find
> that a bit counterintuitive. Maybe it has a reason?
> 

Hi,

You find the behavior or the commit message counterintuitive? Didn't you
suggest this behavior in your email from "7 Jan 2014 13:01:11 +0100"?


For v3 I will reword the commit message. How about the following:

    ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
    
    Refactor the deletion/update of prefix routes when removing an
    address. Now, consider IFA_F_NOPREFIXROUTE and if there is an address
    present with this flag, to not cleanup the route. Instead, assume
    that userspace is taking care of this prefix.
    
    Also perform the same cleanup, when userspace changes an existing address
    to add NOPREFIXROUTE to an address that didn't have this flag. We do this
    because when the address was added, a prefix route was created for it.
    Since the user now wants to handle this route by himself, we remove it again.
    
    As before, a prefix route only gets removed, if there is no address
    that might need it. Or, if there are only non-permanent addresses,
    update the lifetime of the route.


ciao,
Thomas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH net-next] net-gre-gro: Add GRE support to the GRO stack
From: H.K. Jerry Chu @ 2014-01-07 18:23 UTC (permalink / raw)
  To: edumazet, herbert, ogerlitz; +Cc: davem, netdev, Jerry Chu

From: Jerry Chu <hkchu@google.com>

This patch built on top of Commit 299603e8370a93dd5d8e8d800f0dff1ce2c53d36
("net-gro: Prepare GRO stack for the upcoming tunneling support") to add
the support of the standard GRE (RFC1701/RFC2784/RFC2890) to the GRO
stack. It also serves as an example for supporting other encapsulation
protocols in the GRO stack in the future.

The patch supports version 0 and all the flags (key, csum, seq#) but
will flush any pkt with the S (seq#) flag. This is because the S flag
is not support by GSO, and a GRO pkt may end up in the forwarding path,
thus requiring GSO support to break it up correctly.

Currently the "packet_offload" structure only contains L3 (ETH_P_IP/
ETH_P_IPV6) GRO offload support so the encapped pkts are limited to
IP pkts (i.e., w/o L2 hdr). But support for other protocol type can
be easily added, so is the support for GRE variations like NVGRE.

The patch also support csum offload. Specifically if the csum flag is on
and the h/w is capable of checksumming the payload (CHECKSUM_COMPLETE),
the code will take advantage of the csum computed by the h/w when
validating the GRE csum.

Note that commit 60769a5dcd8755715c7143b4571d5c44f01796f1 "ipv4: gre:
add GRO capability" already introduces GRO capability to IPv4 GRE
tunnels, using the gro_cells infrastructure. But GRO is done after
GRE hdr has been removed (i.e., decapped). The following patch applies
GRO when pkts first come in (before hitting the GRE tunnel code). There
is some performance advantage for applying GRO as early as possible.
Also this approach is transparent to other subsystem like Open vSwitch
where GRE decap is handled outside of the IP stack hence making it
harder for the gro_cells stuff to apply. On the other hand, some NICs
are still not capable of hashing on the inner hdr of a GRE pkt (RSS).
In that case the GRO processing of pkts from the same remote host will
all happen on the same CPU and the performance may be suboptimal.

I'm including some rough preliminary performance numbers below. Note
that the performance will be highly dependent on traffic load, mix as
usual. Moreover it also depends on NIC offload features hence the
following is by no means a comprehesive study. Local testing and tuning
will be needed to decide the best setting.

All tests spawned 50 copies of netperf TCP_STREAM and ran for 30 secs.
(super_netperf 50 -H 192.168.1.18 -l 30)

An IP GRE tunnel with only the key flag on (e.g., ip tunnel add gre1
mode gre local 10.246.17.18 remote 10.246.17.17 ttl 255 key 123)
is configured.

The GRO support for pkts AFTER decap are controlled through the device
feature of the GRE device (e.g., ethtool -K gre1 gro on/off).

1.1 ethtool -K gre1 gro off; ethtool -K eth0 gro off
thruput: 9.16Gbps
CPU utilization: 19%

1.2 ethtool -K gre1 gro on; ethtool -K eth0 gro off
thruput: 5.9Gbps
CPU utilization: 15%

1.3 ethtool -K gre1 gro off; ethtool -K eth0 gro on
thruput: 9.26Gbps
CPU utilization: 12-13%

1.4 ethtool -K gre1 gro on; ethtool -K eth0 gro on
thruput: 9.26Gbps
CPU utilization: 10%

The following tests were performed on a different NIC that is capable of
csum offload. I.e., the h/w is capable of computing IP payload csum
(CHECKSUM_COMPLETE).

2.1 ethtool -K gre1 gro on (hence will use gro_cells)

2.1.1 ethtool -K eth0 gro off; csum offload disabled
thruput: 8.53Gbps
CPU utilization: 9%

2.1.2 ethtool -K eth0 gro off; csum offload enabled
thruput: 8.97Gbps
CPU utilization: 7-8%

2.1.3 ethtool -K eth0 gro on; csum offload disabled
thruput: 8.83Gbps
CPU utilization: 5-6%

2.1.4 ethtool -K eth0 gro on; csum offload enabled
thruput: 8.98Gbps
CPU utilization: 5%

2.2 ethtool -K gre1 gro off

2.2.1 ethtool -K eth0 gro off; csum offload disabled
thruput: 5.93Gbps
CPU utilization: 9%

2.2.2 ethtool -K eth0 gro off; csum offload enabled
thruput: 5.62Gbps
CPU utilization: 8%

2.2.3 ethtool -K eth0 gro on; csum offload disabled
thruput: 7.69Gbps
CPU utilization: 8%

2.2.4 ethtool -K eth0 gro on; csum offload enabled
thruput: 8.96Gbps
CPU utilization: 5-6%

Signed-off-by: H.K. Jerry Chu <hkchu@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/netdevice.h |  18 +++++-
 net/core/dev.c            |  26 ++++++++
 net/ipv4/af_inet.c        |  10 ++-
 net/ipv4/gre_offload.c    | 160 ++++++++++++++++++++++++++++++++++++++++++++++
 net/ipv4/tcp_offload.c    |   7 +-
 net/ipv6/ip6_offload.c    |   2 +-
 6 files changed, 216 insertions(+), 7 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d9c961a..a2a70cc 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1632,7 +1632,10 @@ struct napi_gro_cb {
 	int data_offset;
 
 	/* This is non-zero if the packet cannot be merged with the new skb. */
-	int flush;
+	u16	flush;
+
+	/* Save the IP ID here and check when we get to the transport layer */
+	u16	flush_id;
 
 	/* Number of segments aggregated. */
 	u16	count;
@@ -1651,6 +1654,9 @@ struct napi_gro_cb {
 	/* Used in ipv6_gro_receive() */
 	int	proto;
 
+	/* used to support CHECKSUM_COMPLETE for tunneling protocols */
+	__wsum	csum;
+
 	/* used in skb_gro_receive() slow path */
 	struct sk_buff *last;
 };
@@ -1900,6 +1906,14 @@ static inline void *skb_gro_network_header(struct sk_buff *skb)
 	       skb_network_offset(skb);
 }
 
+static inline void skb_gro_postpull_rcsum(struct sk_buff *skb,
+					const void *start, unsigned int len)
+{
+	if (skb->ip_summed == CHECKSUM_COMPLETE)
+		NAPI_GRO_CB(skb)->csum = csum_sub(NAPI_GRO_CB(skb)->csum,
+						  csum_partial(start, len, 0));
+}
+
 static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 				  unsigned short type,
 				  const void *daddr, const void *saddr,
@@ -2440,6 +2454,8 @@ gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb);
 void napi_gro_flush(struct napi_struct *napi, bool flush_old);
 struct sk_buff *napi_get_frags(struct napi_struct *napi);
 gro_result_t napi_gro_frags(struct napi_struct *napi);
+struct packet_offload *gro_find_receive_by_type(__be16 type);
+struct packet_offload *gro_find_complete_by_type(__be16 type);
 
 static inline void napi_free_frags(struct napi_struct *napi)
 {
diff --git a/net/core/dev.c b/net/core/dev.c
index e5e23d7..ca520ed 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3846,6 +3846,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 
 	skb_gro_reset_offset(skb);
 	gro_list_prepare(napi, skb);
+	NAPI_GRO_CB(skb)->csum = skb->csum; /* Needed for CHECKSUM_COMPLETE */
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(ptype, head, list) {
@@ -3922,6 +3923,31 @@ normal:
 	goto pull;
 }
 
+struct packet_offload *gro_find_receive_by_type(__be16 type)
+{
+	struct list_head *offload_head = &offload_base;
+	struct packet_offload *ptype;
+
+	list_for_each_entry_rcu(ptype, offload_head, list) {
+		if (ptype->type != type || !ptype->callbacks.gro_receive)
+			continue;
+		return ptype;
+	}
+	return NULL;
+}
+
+struct packet_offload *gro_find_complete_by_type(__be16 type)
+{
+	struct list_head *offload_head = &offload_base;
+	struct packet_offload *ptype;
+
+	list_for_each_entry_rcu(ptype, offload_head, list) {
+		if (ptype->type != type || !ptype->callbacks.gro_complete)
+			continue;
+		return ptype;
+	}
+	return NULL;
+}
 
 static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
 {
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index b8bc1a3..6268a47 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1391,9 +1391,15 @@ static struct sk_buff **inet_gro_receive(struct sk_buff **head,
 		NAPI_GRO_CB(p)->flush |=
 			(iph->ttl ^ iph2->ttl) |
 			(iph->tos ^ iph2->tos) |
-			(__force int)((iph->frag_off ^ iph2->frag_off) & htons(IP_DF)) |
-			((u16)(ntohs(iph2->id) + NAPI_GRO_CB(p)->count) ^ id);
+			((iph->frag_off ^ iph2->frag_off) & htons(IP_DF));
 
+		/* Save the IP ID check to be included later when we get to
+		 * the transport layer so only the inner most IP ID is checked.
+		 * This is because some GSO/TSO implementations do not
+		 * correctly increment the IP ID for the outer hdrs.
+		 */
+		NAPI_GRO_CB(p)->flush_id =
+			    ((u16)(ntohs(iph2->id) + NAPI_GRO_CB(p)->count) ^ id);
 		NAPI_GRO_CB(p)->flush |= flush;
 	}
 
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 9138cfb..746a7b1 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -116,10 +116,170 @@ out:
 	return segs;
 }
 
+/* Compute the whole skb csum in s/w and store it, then verify GRO csum
+ * starting from gro_offset.
+ */
+static __sum16 gro_skb_checksum(struct sk_buff *skb)
+{
+	__sum16 sum;
+
+	skb->csum = skb_checksum(skb, 0, skb->len, 0);
+	NAPI_GRO_CB(skb)->csum = csum_sub(skb->csum,
+		csum_partial(skb->data, skb_gro_offset(skb), 0));
+	sum = csum_fold(NAPI_GRO_CB(skb)->csum);
+	if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE)) {
+		if (unlikely(!sum))
+			netdev_rx_csum_fault(skb->dev);
+	} else
+		skb->ip_summed = CHECKSUM_COMPLETE;
+
+	return sum;
+}
+
+static struct sk_buff **gre_gro_receive(struct sk_buff **head,
+					struct sk_buff *skb)
+{
+	struct sk_buff **pp = NULL;
+	struct sk_buff *p;
+	const struct gre_base_hdr *greh;
+	unsigned int hlen, grehlen;
+	unsigned int off;
+	int flush = 1;
+	struct packet_offload *ptype;
+	__be16 type;
+
+	off = skb_gro_offset(skb);
+	hlen = off + sizeof(*greh);
+	greh = skb_gro_header_fast(skb, off);
+	if (skb_gro_header_hard(skb, hlen)) {
+		greh = skb_gro_header_slow(skb, hlen, off);
+		if (unlikely(!greh))
+			goto out;
+	}
+
+	/* Only support version 0 and K (key), C (csum) flags. Note that
+	 * although the support for the S (seq#) flag can be added easily
+	 * for GRO, this is problematic for GSO hence can not be enabled
+	 * here because a GRO pkt may end up in the forwarding path, thus
+	 * requiring GSO support to break it up correctly.
+	 */
+	if ((greh->flags & ~(GRE_KEY|GRE_CSUM)) != 0)
+		goto out;
+
+	type = greh->protocol;
+
+	rcu_read_lock();
+	ptype = gro_find_receive_by_type(type);
+	if (ptype == NULL)
+		goto out_unlock;
+
+	grehlen = GRE_HEADER_SECTION;
+
+	if (greh->flags & GRE_KEY)
+		grehlen += GRE_HEADER_SECTION;
+
+	if (greh->flags & GRE_CSUM)
+		grehlen += GRE_HEADER_SECTION;
+
+	hlen = off + grehlen;
+	if (skb_gro_header_hard(skb, hlen)) {
+		greh = skb_gro_header_slow(skb, hlen, off);
+		if (unlikely(!greh))
+			goto out_unlock;
+	}
+	if (greh->flags & GRE_CSUM) { /* Need to verify GRE csum first */
+		__sum16 csum = 0;
+
+		if (skb->ip_summed == CHECKSUM_COMPLETE)
+			csum = csum_fold(NAPI_GRO_CB(skb)->csum);
+		/* Don't trust csum error calculated/reported by h/w */
+		if (skb->ip_summed == CHECKSUM_NONE || csum != 0)
+			csum = gro_skb_checksum(skb);
+
+		/* GRE CSUM is the 1's complement of the 1's complement sum
+		 * of the GRE hdr plus payload so it should add up to 0xffff
+		 * (and 0 after csum_fold()) just like the IPv4 hdr csum.
+		 */
+		if (csum)
+			goto out_unlock;
+	}
+	flush = 0;
+
+	for (p = *head; p; p = p->next) {
+		const struct gre_base_hdr *greh2;
+
+		if (!NAPI_GRO_CB(p)->same_flow)
+			continue;
+
+		/* The following checks are needed to ensure only pkts
+		 * from the same tunnel are considered for aggregation.
+		 * The criteria for "the same tunnel" includes:
+		 * 1) same version (we only support version 0 here)
+		 * 2) same protocol (we only support ETH_P_IP for now)
+		 * 3) same set of flags
+		 * 4) same key if the key field is present.
+		 */
+		greh2 = (struct gre_base_hdr *)(p->data + off);
+
+		if (greh2->flags != greh->flags ||
+		    greh2->protocol != greh->protocol) {
+			NAPI_GRO_CB(p)->same_flow = 0;
+			continue;
+		}
+		if (greh->flags & GRE_KEY) {
+			/* compare keys */
+			if (*(__be32 *)(greh2+1) != *(__be32 *)(greh+1)) {
+				NAPI_GRO_CB(p)->same_flow = 0;
+				continue;
+			}
+		}
+	}
+
+	skb_gro_pull(skb, grehlen);
+
+	/* Adjusted NAPI_GRO_CB(skb)->csum after skb_gro_pull()*/
+	skb_gro_postpull_rcsum(skb, greh, grehlen);
+
+	pp = ptype->callbacks.gro_receive(head, skb);
+
+out_unlock:
+	rcu_read_unlock();
+out:
+	NAPI_GRO_CB(skb)->flush |= flush;
+
+	return pp;
+}
+
+int gre_gro_complete(struct sk_buff *skb, int nhoff)
+{
+	struct gre_base_hdr *greh = (struct gre_base_hdr *)(skb->data + nhoff);
+	struct packet_offload *ptype;
+	unsigned int grehlen = sizeof(*greh);
+	int err = -ENOENT;
+	__be16 type;
+
+	type = greh->protocol;
+	if (greh->flags & GRE_KEY)
+		grehlen += GRE_HEADER_SECTION;
+
+	if (greh->flags & GRE_CSUM)
+		grehlen += GRE_HEADER_SECTION;
+
+	rcu_read_lock();
+	ptype = gro_find_complete_by_type(type);
+	if (ptype != NULL)
+		err = ptype->callbacks.gro_complete(skb, nhoff + grehlen);
+
+	rcu_read_unlock();
+	return err;
+}
+
 static const struct net_offload gre_offload = {
 	.callbacks = {
 		.gso_send_check = gre_gso_send_check,
 		.gso_segment = gre_gso_segment,
+		.gro_receive = gre_gro_receive,
+		.gro_complete = gre_gro_complete,
 	},
 };
 
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 2658a27..771a395 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -197,7 +197,8 @@ struct sk_buff **tcp_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 	goto out_check_final;
 
 found:
-	flush = NAPI_GRO_CB(p)->flush;
+	/* Include the IP ID check below from the inner most IP hdr */
+	flush = NAPI_GRO_CB(p)->flush | NAPI_GRO_CB(p)->flush_id;
 	flush |= (__force int)(flags & TCP_FLAG_CWR);
 	flush |= (__force int)((flags ^ tcp_flag_word(th2)) &
 		  ~(TCP_FLAG_CWR | TCP_FLAG_FIN | TCP_FLAG_PSH));
@@ -230,7 +231,7 @@ out_check_final:
 		pp = head;
 
 out:
-	NAPI_GRO_CB(skb)->flush |= flush;
+	NAPI_GRO_CB(skb)->flush |= (flush != 0);
 
 	return pp;
 }
@@ -280,7 +281,7 @@ static struct sk_buff **tcp4_gro_receive(struct sk_buff **head, struct sk_buff *
 	if (NAPI_GRO_CB(skb)->flush)
 		goto skip_csum;
 
-	wsum = skb->csum;
+	wsum = NAPI_GRO_CB(skb)->csum;
 
 	switch (skb->ip_summed) {
 	case CHECKSUM_NONE:
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 6fb4162..1e8683b 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -190,7 +190,7 @@ static struct sk_buff **ipv6_gro_receive(struct sk_buff **head,
 	unsigned int nlen;
 	unsigned int hlen;
 	unsigned int off;
-	int flush = 1;
+	u16 flush = 1;
 	int proto;
 	__wsum csum;
 
-- 
1.8.5.1

^ permalink raw reply related

* Re: [PATCH -next] qlcnic: use vzalloc() instead of vmalloc()/memset(0)
From: Jitendra Kalsaria @ 2014-01-07 18:22 UTC (permalink / raw)
  To: Wei Yongjun, Himanshu Madhani, Rajesh Borundia, Shahed Shaikh,
	Sony Chacko, Sucheta Chakraborty
  Cc: yongjun_wei@trendmicro.com.cn, Dept-Eng Linux Driver, netdev
In-Reply-To: <CAPgLHd9kdVxAPmxcyZeRpJC7_s8HGyefA+CoYkhhz9pEM2xGKg@mail.gmail.com>


On 1/7/14 5:48 AM, "Wei Yongjun" <weiyj.lk@gmail.com> wrote:

>From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
>Use vzalloc() instead of vmalloc() and memset(0).
>
>Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>---
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)

Acked-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>
>diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
>b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
>index b529667..16912e2 100644
>--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
>+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
>@@ -935,11 +935,10 @@ static ssize_t qlcnic_sysfs_read_pci_config(struct
>file *file,
> 		return QL_STATUS_INVALID_PARAM;
> 
> 	pci_info_sz = pci_func_count * sizeof(*pci_info);
>-	pci_info = vmalloc(pci_info_sz);
>+	pci_info = vzalloc(pci_info_sz);
> 	if (!pci_info)
> 		return -ENOMEM;
> 
>-	memset(pci_info, 0, pci_info_sz);
> 	memset(buf, 0, pci_cfg_sz);
> 	pci_cfg = (struct qlcnic_pci_func_cfg *)buf;
> 
>

Thanks,
Jiten

^ permalink raw reply

* Use of 'SIOCDEVPRIVATE' in ethernet drivers.
From: Giri Reddy @ 2014-01-07 18:21 UTC (permalink / raw)
  To: netdev

I'm implementing a new interface for upgrading the firmware for our next
generation of Qlogic drivers. We need to transfer control information
between userspace and driver, in addition to the firmware payload. I'm
considering using 'SIOCDEVPRIVATE' ioctl interface to pass control
information between userland and driver.

I also see comments in the source (include/linux/sockios.h) that
'SIOCDEVPRIVATE' will be deprecated 2.5.x. What is the current status of
deprecation in the latest versions of the kernel?

Are we still allowed to use 'SIOCDEVPRIVATE' in drivers if there the
deprecation efforts are on hold.

Any input will be appreciated.

Thanks.
Giri
Qlogic

________________________________

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

^ permalink raw reply

* Re: [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Tom Herbert @ 2014-01-07 18:08 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Jerry Chu, Eric Dumazet, Herbert Xu, Linux Netdev List,
	David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <1389108594-665-4-git-send-email-ogerlitz@mellanox.com>

On Tue, Jan 7, 2014 at 7:29 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Add gro handlers for vxlan using the udp gro infrastructure
>
> On my setup, which is net-next (now with the mlx4 vxlan offloads patches) --
> for single TCP session that goes through vxlan tunneling I got nice improvement
> from 6.8Gbs to 11.5Gbs
>
> --> UDP/VXLAN GRO disabled
> $ netperf  -H 192.168.52.147 -c -C
>
> $ netperf -t TCP_STREAM -H 192.168.52.147 -c -C
> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.52.147 () port 0 AF_INET
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
>
>  87380  65536  65536    10.00      6799.75   12.54    24.79    0.604   1.195
>
> --> UDP/VXLAN GRO enabled
>
> $ netperf -t TCP_STREAM -H 192.168.52.147 -c -C
> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.52.147 () port 0 AF_INET
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
>
>  87380  65536  65536    10.00      11562.72   24.90    20.34    0.706   0.577
>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
>  drivers/net/vxlan.c |  101 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 481f85d..b51823b 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -40,6 +40,7 @@
>  #include <net/net_namespace.h>
>  #include <net/netns/generic.h>
>  #include <net/vxlan.h>
> +#include <net/protocol.h>
>  #if IS_ENABLED(CONFIG_IPV6)
>  #include <net/ipv6.h>
>  #include <net/addrconf.h>
> @@ -554,6 +555,98 @@ static int vxlan_fdb_append(struct vxlan_fdb *f,
>         return 1;
>  }
>
> +static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff *skb)
> +{
> +       struct sk_buff *p, **pp = NULL;
> +       struct vxlanhdr *vh, *vh2;
> +       struct ethhdr *eh;
> +       unsigned int hlen, off, off_eth;
> +       const struct packet_offload *ptype;
> +       __be16 type;
> +       int flush = 1;
> +
> +       off  = skb_gro_offset(skb);
> +       hlen = off + sizeof(*vh);
> +       vh   = skb_gro_header_fast(skb, off);
> +       if (skb_gro_header_hard(skb, hlen)) {
> +               vh = skb_gro_header_slow(skb, hlen, off);
> +               if (unlikely(!vh))
> +                       goto out;
> +       }
> +
> +       flush = 0;
> +
> +       for (p = *head; p; p = p->next) {
> +               if (!NAPI_GRO_CB(p)->same_flow)
> +                       continue;
> +
> +               vh2 = (struct vxlanhdr   *)(p->data + off);
> +               if (vh->vx_vni ^ vh2->vx_vni) {

Why ^ instead of != ?

> +                       NAPI_GRO_CB(p)->same_flow = 0;
> +                       continue;
> +               }
> +               goto found;
> +       }
> +
> +found:
> +       skb_gro_pull(skb, sizeof(struct vxlanhdr)); /* pull vxlan header */
> +
> +       off_eth = skb_gro_offset(skb);
> +       hlen = off_eth + sizeof(*eh);
> +       eh   = skb_gro_header_fast(skb, off_eth);
> +       if (skb_gro_header_hard(skb, hlen)) {
> +               eh = skb_gro_header_slow(skb, hlen, off_eth);
> +               if (unlikely(!eh))
> +                       goto out;
> +       }
> +       type = eh->h_proto;
> +
> +       rcu_read_lock();
> +       ptype = gro_find_receive_by_type(type);
> +       if (ptype == NULL) {
> +               flush = 1;
> +               goto out_unlock;
> +       }
> +
> +       skb_gro_pull(skb, sizeof(*eh)); /* pull inner eth header */
> +       pp = ptype->callbacks.gro_receive(head, skb);
> +
> +out_unlock:
> +       rcu_read_unlock();
> +out:
> +       NAPI_GRO_CB(skb)->flush |= flush;
> +
> +       return pp;
> +}
> +
> +static int vxlan_gro_complete(struct sk_buff *skb, int nhoff)
> +{
> +       struct ethhdr *eh;
> +       struct packet_offload *ptype;
> +       __be16 type;
> +       /* 22 = 8 bytes for the vlxan header + 14 bytes for the inner eth header */
> +       int vxlan_len  = 22;
> +       int err = -ENOSYS;
> +
> +       eh = (struct ethhdr *)(skb->data + nhoff + sizeof (struct vxlanhdr));
> +       type = eh->h_proto;
> +
> +       rcu_read_lock();
> +       ptype = gro_find_complete_by_type(type);
> +       if (ptype != NULL)
> +               err = ptype->callbacks.gro_complete(skb, nhoff + vxlan_len);
> +
> +       rcu_read_unlock();
> +       return err;
> +}
> +
> +static const struct net_offload vxlan_offload = {
> +       .callbacks = {
> +               .gro_receive  = vxlan_gro_receive,
> +               .gro_complete = vxlan_gro_complete,
> +       },
> +};
> +
>  /* Notify netdevs that UDP port started listening */
>  static void vxlan_notify_add_rx_port(struct sock *sk)
>  {
> @@ -568,6 +661,8 @@ static void vxlan_notify_add_rx_port(struct sock *sk)
>                         dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
>                                                             port);
>         }
> +       if (sa_family == AF_INET)
> +               udp_add_offload(&vxlan_offload, port);
>         rcu_read_unlock();
>  }
>
> @@ -585,6 +680,8 @@ static void vxlan_notify_del_rx_port(struct sock *sk)
>                         dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
>                                                             port);
>         }
> +       if (sa_family == AF_INET)
> +               udp_del_offload(&vxlan_offload, port);
>         rcu_read_unlock();
>  }
>
> @@ -1125,8 +1222,8 @@ static void vxlan_rcv(struct vxlan_sock *vs,
>          * leave the CHECKSUM_UNNECESSARY, the device checksummed it
>          * for us. Otherwise force the upper layers to verify it.
>          */
> -       if (skb->ip_summed != CHECKSUM_UNNECESSARY || !skb->encapsulation ||
> -           !(vxlan->dev->features & NETIF_F_RXCSUM))
> +       if ((skb->ip_summed != CHECKSUM_UNNECESSARY && skb->ip_summed != CHECKSUM_PARTIAL) ||
> +           !skb->encapsulation || !(vxlan->dev->features & NETIF_F_RXCSUM))
>                 skb->ip_summed = CHECKSUM_NONE;
>
>         skb->encapsulation = 0;
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox