Netdev List
 help / color / mirror / Atom feed
* [patch net-next 2/8] act_ife: Change to use ife module
From: Jiri Pirko @ 2016-11-10 11:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
	geert+renesas, stephen, xiyou.wangcong, linux, roopa
In-Reply-To: <1478776988-5400-1-git-send-email-jiri@resnulli.us>

From: Yotam Gigi <yotamg@mellanox.com>

Use the encode/decode functionality from the ife module instead of using
implementation inside the act_ife.

Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/tc_act/tc_ife.h        |   3 -
 include/uapi/linux/tc_act/tc_ife.h |  10 +---
 net/sched/Kconfig                  |   1 +
 net/sched/act_ife.c                | 109 +++++++++++--------------------------
 4 files changed, 34 insertions(+), 89 deletions(-)

diff --git a/include/net/tc_act/tc_ife.h b/include/net/tc_act/tc_ife.h
index 9fd2bea0..30ba459 100644
--- a/include/net/tc_act/tc_ife.h
+++ b/include/net/tc_act/tc_ife.h
@@ -6,7 +6,6 @@
 #include <linux/rtnetlink.h>
 #include <linux/module.h>
 
-#define IFE_METAHDRLEN 2
 struct tcf_ife_info {
 	struct tc_action common;
 	u8 eth_dst[ETH_ALEN];
@@ -45,8 +44,6 @@ struct tcf_meta_ops {
 
 int ife_get_meta_u32(struct sk_buff *skb, struct tcf_meta_info *mi);
 int ife_get_meta_u16(struct sk_buff *skb, struct tcf_meta_info *mi);
-int ife_tlv_meta_encode(void *skbdata, u16 attrtype, u16 dlen,
-			const void *dval);
 int ife_alloc_meta_u32(struct tcf_meta_info *mi, void *metaval, gfp_t gfp);
 int ife_alloc_meta_u16(struct tcf_meta_info *mi, void *metaval, gfp_t gfp);
 int ife_check_meta_u32(u32 metaval, struct tcf_meta_info *mi);
diff --git a/include/uapi/linux/tc_act/tc_ife.h b/include/uapi/linux/tc_act/tc_ife.h
index cd18360..7c28178 100644
--- a/include/uapi/linux/tc_act/tc_ife.h
+++ b/include/uapi/linux/tc_act/tc_ife.h
@@ -3,6 +3,7 @@
 
 #include <linux/types.h>
 #include <linux/pkt_cls.h>
+#include <linux/ife.h>
 
 #define TCA_ACT_IFE 25
 /* Flag bits for now just encoding/decoding; mutually exclusive */
@@ -28,13 +29,4 @@ enum {
 };
 #define TCA_IFE_MAX (__TCA_IFE_MAX - 1)
 
-#define IFE_META_SKBMARK 1
-#define IFE_META_HASHID 2
-#define	IFE_META_PRIO 3
-#define	IFE_META_QMAP 4
-#define	IFE_META_TCINDEX 5
-/*Can be overridden at runtime by module option*/
-#define	__IFE_META_MAX 6
-#define IFE_META_MAX (__IFE_META_MAX - 1)
-
 #endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 87956a7..24f7cac 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -763,6 +763,7 @@ config NET_ACT_SKBMOD
 config NET_ACT_IFE
         tristate "Inter-FE action based on IETF ForCES InterFE LFB"
         depends on NET_CLS_ACT
+        select NET_IFE
         ---help---
 	  Say Y here to allow for sourcing and terminating metadata
 	  For details refer to netdev01 paper:
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 95c463c..5c2478a 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -32,6 +32,7 @@
 #include <uapi/linux/tc_act/tc_ife.h>
 #include <net/tc_act/tc_ife.h>
 #include <linux/etherdevice.h>
+#include <net/ife.h>
 
 #define IFE_TAB_MASK 15
 
@@ -46,23 +47,6 @@ static const struct nla_policy ife_policy[TCA_IFE_MAX + 1] = {
 	[TCA_IFE_TYPE] = { .type = NLA_U16},
 };
 
-/* Caller takes care of presenting data in network order
-*/
-int ife_tlv_meta_encode(void *skbdata, u16 attrtype, u16 dlen, const void *dval)
-{
-	u32 *tlv = (u32 *)(skbdata);
-	u16 totlen = nla_total_size(dlen);	/*alignment + hdr */
-	char *dptr = (char *)tlv + NLA_HDRLEN;
-	u32 htlv = attrtype << 16 | (dlen + NLA_HDRLEN);
-
-	*tlv = htonl(htlv);
-	memset(dptr, 0, totlen - NLA_HDRLEN);
-	memcpy(dptr, dval, dlen);
-
-	return totlen;
-}
-EXPORT_SYMBOL_GPL(ife_tlv_meta_encode);
-
 int ife_encode_meta_u16(u16 metaval, void *skbdata, struct tcf_meta_info *mi)
 {
 	u16 edata = 0;
@@ -637,69 +621,60 @@ int find_decode_metaid(struct sk_buff *skb, struct tcf_ife_info *ife,
 	return 0;
 }
 
-struct ifeheadr {
-	__be16 metalen;
-	u8 tlv_data[];
-};
-
-struct meta_tlvhdr {
-	__be16 type;
-	__be16 len;
-};
-
 static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
 			  struct tcf_result *res)
 {
 	struct tcf_ife_info *ife = to_ife(a);
+	u32 at = G_TC_AT(skb->tc_verd);
 	int action = ife->tcf_action;
-	struct ifeheadr *ifehdr = (struct ifeheadr *)skb->data;
-	int ifehdrln = (int)ifehdr->metalen;
-	struct meta_tlvhdr *tlv = (struct meta_tlvhdr *)(ifehdr->tlv_data);
+	u8 *ifehdr_end;
+	u8 *tlv_data;
+	u16 metalen;
 
 	spin_lock(&ife->tcf_lock);
 	bstats_update(&ife->tcf_bstats, skb);
 	tcf_lastuse_update(&ife->tcf_tm);
 	spin_unlock(&ife->tcf_lock);
 
-	ifehdrln = ntohs(ifehdrln);
-	if (unlikely(!pskb_may_pull(skb, ifehdrln))) {
+	if (!(at & AT_EGRESS))
+		skb_push(skb, skb->dev->hard_header_len);
+
+	tlv_data = ife_decode(skb, &metalen);
+	if (unlikely(!tlv_data)) {
 		spin_lock(&ife->tcf_lock);
 		ife->tcf_qstats.drops++;
 		spin_unlock(&ife->tcf_lock);
 		return TC_ACT_SHOT;
 	}
 
-	skb_set_mac_header(skb, ifehdrln);
-	__skb_pull(skb, ifehdrln);
-	skb->protocol = eth_type_trans(skb, skb->dev);
-	ifehdrln -= IFE_METAHDRLEN;
-
-	while (ifehdrln > 0) {
-		u8 *tlvdata = (u8 *)tlv;
-		u16 mtype = tlv->type;
-		u16 mlen = tlv->len;
-		u16 alen;
+	ifehdr_end = tlv_data + metalen;
+	for (; tlv_data < ifehdr_end; tlv_data = ife_tlv_meta_next(tlv_data)) {
+		u8 *curr_data;
+		u16 mtype;
+		u16 dlen;
 
-		mtype = ntohs(mtype);
-		mlen = ntohs(mlen);
-		alen = NLA_ALIGN(mlen);
+		curr_data = ife_tlv_meta_decode(tlv_data, &mtype, &dlen, NULL);
 
-		if (find_decode_metaid(skb, ife, mtype, (mlen - NLA_HDRLEN),
-				       (void *)(tlvdata + NLA_HDRLEN))) {
+		if (find_decode_metaid(skb, ife, mtype, dlen, curr_data)) {
 			/* abuse overlimits to count when we receive metadata
 			 * but dont have an ops for it
 			 */
-			pr_info_ratelimited("Unknown metaid %d alnlen %d\n",
-					    mtype, mlen);
+			pr_info_ratelimited("Unknown metaid %d dlen %d\n",
+					    mtype, dlen);
 			ife->tcf_qstats.overlimits++;
 		}
+	}
 
-		tlvdata += alen;
-		ifehdrln -= alen;
-		tlv = (struct meta_tlvhdr *)tlvdata;
+	if (WARN_ON(tlv_data != ifehdr_end)) {
+		spin_lock(&ife->tcf_lock);
+		ife->tcf_qstats.drops++;
+		spin_unlock(&ife->tcf_lock);
+		return TC_ACT_SHOT;
 	}
 
+	skb->protocol = eth_type_trans(skb, skb->dev);
 	skb_reset_network_header(skb);
+
 	return action;
 }
 
@@ -727,7 +702,6 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	struct tcf_ife_info *ife = to_ife(a);
 	int action = ife->tcf_action;
 	struct ethhdr *oethh;	/* outer ether header */
-	struct ethhdr *iethh;	/* inner eth header */
 	struct tcf_meta_info *e;
 	/*
 	   OUTERHDR:TOTMETALEN:{TLVHDR:Metadatum:TLVHDR..}:ORIGDATA
@@ -735,10 +709,11 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	 */
 	u16 metalen = ife_get_sz(skb, ife);
 	int hdrm = metalen + skb->dev->hard_header_len + IFE_METAHDRLEN;
-	unsigned int skboff = skb->dev->hard_header_len;
+	unsigned int skboff = 0;
 	u32 at = G_TC_AT(skb->tc_verd);
 	int new_len = skb->len + hdrm;
 	bool exceed_mtu = false;
+	void *ife_meta;
 	int err;
 
 	if (at & AT_EGRESS) {
@@ -766,27 +741,10 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 		return TC_ACT_SHOT;
 	}
 
-	err = skb_cow_head(skb, hdrm);
-	if (unlikely(err)) {
-		ife->tcf_qstats.drops++;
-		spin_unlock(&ife->tcf_lock);
-		return TC_ACT_SHOT;
-	}
-
 	if (!(at & AT_EGRESS))
 		skb_push(skb, skb->dev->hard_header_len);
 
-	iethh = (struct ethhdr *)skb->data;
-	__skb_push(skb, hdrm);
-	memcpy(skb->data, iethh, skb->mac_len);
-	skb_reset_mac_header(skb);
-	oethh = eth_hdr(skb);
-
-	/*total metadata length */
-	metalen += IFE_METAHDRLEN;
-	metalen = htons(metalen);
-	memcpy((skb->data + skboff), &metalen, IFE_METAHDRLEN);
-	skboff += IFE_METAHDRLEN;
+	ife_meta = ife_encode(skb, metalen);
 
 	/* XXX: we dont have a clever way of telling encode to
 	 * not repeat some of the computations that are done by
@@ -794,7 +752,7 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	 */
 	list_for_each_entry(e, &ife->metalist, metalist) {
 		if (e->ops->encode) {
-			err = e->ops->encode(skb, (void *)(skb->data + skboff),
+			err = e->ops->encode(skb, (void *)(ife_meta + skboff),
 					     e);
 		}
 		if (err < 0) {
@@ -805,15 +763,12 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 		}
 		skboff += err;
 	}
+	oethh = (struct ethhdr *)skb->data;
 
 	if (!is_zero_ether_addr(ife->eth_src))
 		ether_addr_copy(oethh->h_source, ife->eth_src);
-	else
-		ether_addr_copy(oethh->h_source, iethh->h_source);
 	if (!is_zero_ether_addr(ife->eth_dst))
 		ether_addr_copy(oethh->h_dest, ife->eth_dst);
-	else
-		ether_addr_copy(oethh->h_dest, iethh->h_dest);
 	oethh->h_proto = htons(ife->eth_type);
 
 	if (!(at & AT_EGRESS))
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function
From: Oliver Neukum @ 2016-11-10 11:22 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Kai-Heng Feng, Alan Stern, linux-kernel, linux-usb, netdev,
	Mathias Nyman
In-Reply-To: <87a8d7h9q1.fsf@miraculix.mork.no>

On Thu, 2016-11-10 at 12:09 +0100, Bjørn Mork wrote:
> Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
> > On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <bjorn@mork.no> wrote:
> >> Oliver Neukum <oneukum@suse.com> writes:
> >>
> >>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
> >>>
> >>>> These problems could very well be caused by running at SuperSpeed
> >>>> (USB-3) instead of high speed (USB-2).
> >
> > Yes, it's running at SuperSpeed, on a Kabylake laptop.
> >
> > It does not have this issue on a Broadwell laptop, also running at SuperSpeed.
> 
> Then I must join Oliver, being very surprised by where in the stack you
> attempt to fix the issue.  What you write above indicates a problem in
> pci bridge or usb host controller, doesn't it?

Indeed. And this means we need an XHCI specialist.
Mathias, we have a failure specific to one implementation of XHCI.

	Regards
		Oliver

^ permalink raw reply

* [PATCH] net: bpqether.h: remove if_ether.h guard
From: Baruch Siach @ 2016-11-10 11:21 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: linux-hams, netdev, Baruch Siach

__LINUX_IF_ETHER_H is not defined anywhere, and if_ether.h can keep itself from
double inclusion, though it uses a single underscore prefix.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 include/uapi/linux/bpqether.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/uapi/linux/bpqether.h b/include/uapi/linux/bpqether.h
index a6c35e1a89ad..05865edaefda 100644
--- a/include/uapi/linux/bpqether.h
+++ b/include/uapi/linux/bpqether.h
@@ -5,9 +5,7 @@
  * 	Defines for the BPQETHER pseudo device driver
  */
 
-#ifndef __LINUX_IF_ETHER_H
 #include <linux/if_ether.h>
-#endif
 
 #define SIOCSBPQETHOPT		(SIOCDEVPRIVATE+0)	/* reserved */
 #define SIOCSBPQETHADDR		(SIOCDEVPRIVATE+1)
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH net-next] ipv6: sr: fix IPv6 initialization failure without lwtunnels
From: kbuild test robot @ 2016-11-10 11:20 UTC (permalink / raw)
  To: David Lebrun; +Cc: kbuild-all, netdev, lorenzo, davem, David Lebrun
In-Reply-To: <1478771715-23137-1-git-send-email-david.lebrun@uclouvain.be>

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

Hi David,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/David-Lebrun/ipv6-sr-fix-IPv6-initialization-failure-without-lwtunnels/20161110-175753
config: i386-randconfig-s1-201645 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from net/ipv6/seg6_iptunnel.c:24:0:
   include/net/seg6.h: In function 'seg6_pernet':
   include/net/seg6.h:52:12: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
     return net->ipv6.seg6_data;
               ^~
   net/ipv6/seg6_iptunnel.c: In function 'seg6_output':
>> net/ipv6/seg6_iptunnel.c:289:3: error: implicit declaration of function 'dst_cache_set_ip6' [-Werror=implicit-function-declaration]
      dst_cache_set_ip6(&slwt->cache, dst, &fl6.saddr);
      ^~~~~~~~~~~~~~~~~
   In file included from net/ipv6/seg6_iptunnel.c:24:0:
   include/net/seg6.h: In function 'seg6_pernet':
   include/net/seg6.h:53:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^
   cc1: some warnings being treated as errors

vim +/dst_cache_set_ip6 +289 net/ipv6/seg6_iptunnel.c

6c8702c6 David Lebrun 2016-11-08  273  		struct flowi6 fl6;
6c8702c6 David Lebrun 2016-11-08  274  
6c8702c6 David Lebrun 2016-11-08  275  		fl6.daddr = hdr->daddr;
6c8702c6 David Lebrun 2016-11-08  276  		fl6.saddr = hdr->saddr;
6c8702c6 David Lebrun 2016-11-08  277  		fl6.flowlabel = ip6_flowinfo(hdr);
6c8702c6 David Lebrun 2016-11-08  278  		fl6.flowi6_mark = skb->mark;
6c8702c6 David Lebrun 2016-11-08  279  		fl6.flowi6_proto = hdr->nexthdr;
6c8702c6 David Lebrun 2016-11-08  280  
6c8702c6 David Lebrun 2016-11-08  281  		dst = ip6_route_output(net, NULL, &fl6);
6c8702c6 David Lebrun 2016-11-08  282  		if (dst->error) {
6c8702c6 David Lebrun 2016-11-08  283  			err = dst->error;
6c8702c6 David Lebrun 2016-11-08  284  			dst_release(dst);
6c8702c6 David Lebrun 2016-11-08  285  			goto drop;
6c8702c6 David Lebrun 2016-11-08  286  		}
6c8702c6 David Lebrun 2016-11-08  287  
6c8702c6 David Lebrun 2016-11-08  288  #ifdef CONFIG_DST_CACHE
6c8702c6 David Lebrun 2016-11-08 @289  		dst_cache_set_ip6(&slwt->cache, dst, &fl6.saddr);
6c8702c6 David Lebrun 2016-11-08  290  #endif
6c8702c6 David Lebrun 2016-11-08  291  	}
6c8702c6 David Lebrun 2016-11-08  292  
6c8702c6 David Lebrun 2016-11-08  293  	skb_dst_drop(skb);
6c8702c6 David Lebrun 2016-11-08  294  	skb_dst_set(skb, dst);
6c8702c6 David Lebrun 2016-11-08  295  
6c8702c6 David Lebrun 2016-11-08  296  	return dst_output(net, sk, skb);
6c8702c6 David Lebrun 2016-11-08  297  drop:

:::::: The code at line 289 was first introduced by commit
:::::: 6c8702c60b88651072460f3f4026c7dfe2521d12 ipv6: sr: add support for SRH encapsulation and injection with lwtunnels

:::::: TO: David Lebrun <david.lebrun@uclouvain.be>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27503 bytes --]

^ permalink raw reply

* Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function
From: Bjørn Mork @ 2016-11-10 11:09 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: Oliver Neukum, Alan Stern, linux-kernel, linux-usb, netdev
In-Reply-To: <CAAd53p5W4xpSYsFZ00deQDh-gZBijn_ApreE9NmC=_B_MAQaug@mail.gmail.com>

Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
> On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <bjorn@mork.no> wrote:
>> Oliver Neukum <oneukum@suse.com> writes:
>>
>>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>>>
>>>> These problems could very well be caused by running at SuperSpeed
>>>> (USB-3) instead of high speed (USB-2).
>
> Yes, it's running at SuperSpeed, on a Kabylake laptop.
>
> It does not have this issue on a Broadwell laptop, also running at SuperSpeed.

Then I must join Oliver, being very surprised by where in the stack you
attempt to fix the issue.  What you write above indicates a problem in
pci bridge or usb host controller, doesn't it?


Bjørn

^ permalink raw reply

* [patch net-next] mlxsw: spectrum_router: Add FIB abort warning
From: Jiri Pirko @ 2016-11-10 11:10 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz

From: Jiri Pirko <jiri@mellanox.com>

Add a warning that the abort mechanism was triggered for device.
Also avoid going through the procedure if abort was already done.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 348c773..df31f38 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1877,6 +1877,9 @@ static void mlxsw_sp_router_fib4_abort(struct mlxsw_sp *mlxsw_sp)
 	int i;
 	int err;
 
+	if (mlxsw_sp->router.aborted)
+		return;
+	dev_warn(mlxsw_sp->bus_info->dev, "FIB abort triggered. Note that FIB entries are no longer being offloaded to this device.\n");
 	for (i = 0; i < MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_VRS); i++) {
 		vr = &mlxsw_sp->router.vrs[i];
 		if (!vr->used)
-- 
2.7.4

^ permalink raw reply related

* [patch net 2/2] mlxsw: spectrum_router: Ignore FIB notification events for non-init namespaces
From: Jiri Pirko @ 2016-11-10 11:00 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1478775649-3386-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Since now, the table with same id in multiple netnamespaces were squashed
to a single virtual router. That is not only incorrect, it also causes
error messages when trying to use RALUE register to do double remove
of FIB entries, like this one:

mlxsw_spectrum 0000:03:00.0: EMAD reg access failed (tid=facb831c00007b20,reg_id=8013(ralue),type=write,status=7(bad parameter))

Since we don't allow ports to change namespaces (NETIF_F_NETNS_LOCAL),
and the inrastructure is not yet prepared to handle netnamespaces, just
ignore FIB notification events for non-init namespaces. That is clear to
do since we don't need to offload them.

Fixes: b45f64d16d45 ("mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 2863012..6610672 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1931,6 +1931,9 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
 	struct fib_entry_notifier_info *fen_info = ptr;
 	int err;
 
+	if (fen_info->info.net != &init_net)
+		return NOTIFY_DONE;
+
 	switch (event) {
 	case FIB_EVENT_ENTRY_ADD:
 		err = mlxsw_sp_router_fib4_add(mlxsw_sp, fen_info);
-- 
2.7.4

^ permalink raw reply related

* [patch net 1/2] mlxsw: spectrum_router: Fix handling of neighbour structure
From: Jiri Pirko @ 2016-11-10 11:00 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1478775649-3386-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

__neigh_create function works in a different way than assumed.
It passes "n" as a parameter to ndo_neigh_construct. But this "n" might
be destroyed right away before __neigh_create() returns in case there is
already another neighbour struct in the hashtable with the same dev and
primary key. That is not expected by mlxsw_sp_router_neigh_construct()
and the stored "n" points to freed memory, eventually leading to crash.

Fix this by doing tight 1:1 coupling between neighbour struct and
internal driver neigh_entry. That allows to narrow down the key in
internal driver hashtable to do lookups by "n" only.

Fixes: 6cf3c971dc84 ("mlxsw: spectrum_router: Add private neigh table")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 95 ++++++++--------------
 1 file changed, 34 insertions(+), 61 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 4573da2..2863012 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -600,15 +600,13 @@ static void mlxsw_sp_vrs_fini(struct mlxsw_sp *mlxsw_sp)
 }
 
 struct mlxsw_sp_neigh_key {
-	unsigned char addr[sizeof(struct in6_addr)];
-	struct net_device *dev;
+	struct neighbour *n;
 };
 
 struct mlxsw_sp_neigh_entry {
 	struct rhash_head ht_node;
 	struct mlxsw_sp_neigh_key key;
 	u16 rif;
-	struct neighbour *n;
 	bool offloaded;
 	struct delayed_work dw;
 	struct mlxsw_sp_port *mlxsw_sp_port;
@@ -646,19 +644,15 @@ mlxsw_sp_neigh_entry_remove(struct mlxsw_sp *mlxsw_sp,
 static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work);
 
 static struct mlxsw_sp_neigh_entry *
-mlxsw_sp_neigh_entry_create(const void *addr, size_t addr_len,
-			    struct net_device *dev, u16 rif,
-			    struct neighbour *n)
+mlxsw_sp_neigh_entry_create(struct neighbour *n, u16 rif)
 {
 	struct mlxsw_sp_neigh_entry *neigh_entry;
 
 	neigh_entry = kzalloc(sizeof(*neigh_entry), GFP_ATOMIC);
 	if (!neigh_entry)
 		return NULL;
-	memcpy(neigh_entry->key.addr, addr, addr_len);
-	neigh_entry->key.dev = dev;
+	neigh_entry->key.n = n;
 	neigh_entry->rif = rif;
-	neigh_entry->n = n;
 	INIT_DELAYED_WORK(&neigh_entry->dw, mlxsw_sp_router_neigh_update_hw);
 	INIT_LIST_HEAD(&neigh_entry->nexthop_list);
 	return neigh_entry;
@@ -671,13 +665,11 @@ mlxsw_sp_neigh_entry_destroy(struct mlxsw_sp_neigh_entry *neigh_entry)
 }
 
 static struct mlxsw_sp_neigh_entry *
-mlxsw_sp_neigh_entry_lookup(struct mlxsw_sp *mlxsw_sp, const void *addr,
-			    size_t addr_len, struct net_device *dev)
+mlxsw_sp_neigh_entry_lookup(struct mlxsw_sp *mlxsw_sp, struct neighbour *n)
 {
-	struct mlxsw_sp_neigh_key key = {{ 0 } };
+	struct mlxsw_sp_neigh_key key;
 
-	memcpy(key.addr, addr, addr_len);
-	key.dev = dev;
+	key.n = n;
 	return rhashtable_lookup_fast(&mlxsw_sp->router.neigh_ht,
 				      &key, mlxsw_sp_neigh_ht_params);
 }
@@ -689,26 +681,20 @@ int mlxsw_sp_router_neigh_construct(struct net_device *dev,
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	struct mlxsw_sp_neigh_entry *neigh_entry;
 	struct mlxsw_sp_rif *r;
-	u32 dip;
 	int err;
 
 	if (n->tbl != &arp_tbl)
 		return 0;
 
-	dip = ntohl(*((__be32 *) n->primary_key));
-	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &dip, sizeof(dip),
-						  n->dev);
-	if (neigh_entry) {
-		WARN_ON(neigh_entry->n != n);
+	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
+	if (neigh_entry)
 		return 0;
-	}
 
 	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, n->dev);
 	if (WARN_ON(!r))
 		return -EINVAL;
 
-	neigh_entry = mlxsw_sp_neigh_entry_create(&dip, sizeof(dip), n->dev,
-						  r->rif, n);
+	neigh_entry = mlxsw_sp_neigh_entry_create(n, r->rif);
 	if (!neigh_entry)
 		return -ENOMEM;
 	err = mlxsw_sp_neigh_entry_insert(mlxsw_sp, neigh_entry);
@@ -727,14 +713,11 @@ void mlxsw_sp_router_neigh_destroy(struct net_device *dev,
 	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	struct mlxsw_sp_neigh_entry *neigh_entry;
-	u32 dip;
 
 	if (n->tbl != &arp_tbl)
 		return;
 
-	dip = ntohl(*((__be32 *) n->primary_key));
-	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &dip, sizeof(dip),
-						  n->dev);
+	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
 	if (!neigh_entry)
 		return;
 	mlxsw_sp_neigh_entry_remove(mlxsw_sp, neigh_entry);
@@ -862,7 +845,7 @@ static void mlxsw_sp_router_neighs_update_nh(struct mlxsw_sp *mlxsw_sp)
 		 * is active regardless of the traffic.
 		 */
 		if (!list_empty(&neigh_entry->nexthop_list))
-			neigh_event_send(neigh_entry->n, NULL);
+			neigh_event_send(neigh_entry->key.n, NULL);
 	}
 	rtnl_unlock();
 }
@@ -908,9 +891,9 @@ static void mlxsw_sp_router_probe_unresolved_nexthops(struct work_struct *work)
 	rtnl_lock();
 	list_for_each_entry(neigh_entry, &mlxsw_sp->router.nexthop_neighs_list,
 			    nexthop_neighs_list_node) {
-		if (!(neigh_entry->n->nud_state & NUD_VALID) &&
+		if (!(neigh_entry->key.n->nud_state & NUD_VALID) &&
 		    !list_empty(&neigh_entry->nexthop_list))
-			neigh_event_send(neigh_entry->n, NULL);
+			neigh_event_send(neigh_entry->key.n, NULL);
 	}
 	rtnl_unlock();
 
@@ -927,7 +910,7 @@ static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work)
 {
 	struct mlxsw_sp_neigh_entry *neigh_entry =
 		container_of(work, struct mlxsw_sp_neigh_entry, dw.work);
-	struct neighbour *n = neigh_entry->n;
+	struct neighbour *n = neigh_entry->key.n;
 	struct mlxsw_sp_port *mlxsw_sp_port = neigh_entry->mlxsw_sp_port;
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	char rauht_pl[MLXSW_REG_RAUHT_LEN];
@@ -1030,11 +1013,8 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
 
 		mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 		dip = ntohl(*((__be32 *) n->primary_key));
-		neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp,
-							  &dip,
-							  sizeof(__be32),
-							  dev);
-		if (WARN_ON(!neigh_entry) || WARN_ON(neigh_entry->n != n)) {
+		neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
+		if (WARN_ON(!neigh_entry)) {
 			mlxsw_sp_port_dev_put(mlxsw_sp_port);
 			return NOTIFY_DONE;
 		}
@@ -1343,33 +1323,26 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 				 struct fib_nh *fib_nh)
 {
 	struct mlxsw_sp_neigh_entry *neigh_entry;
-	u32 gwip = ntohl(fib_nh->nh_gw);
 	struct net_device *dev = fib_nh->nh_dev;
 	struct neighbour *n;
 	u8 nud_state;
 
-	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &gwip,
-						  sizeof(gwip), dev);
-	if (!neigh_entry) {
-		__be32 gwipn = htonl(gwip);
-
-		n = neigh_create(&arp_tbl, &gwipn, dev);
+	/* Take a reference of neigh here ensuring that neigh would
+	 * not be detructed before the nexthop entry is finished.
+	 * The reference is taken either in neigh_lookup() or
+	 * in neith_create() in case n is not found.
+	 */
+	n = neigh_lookup(&arp_tbl, &fib_nh->nh_gw, dev);
+	if (!n) {
+		n = neigh_create(&arp_tbl, &fib_nh->nh_gw, dev);
 		if (IS_ERR(n))
 			return PTR_ERR(n);
 		neigh_event_send(n, NULL);
-		neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &gwip,
-							  sizeof(gwip), dev);
-		if (!neigh_entry) {
-			neigh_release(n);
-			return -EINVAL;
-		}
-	} else {
-		/* Take a reference of neigh here ensuring that neigh would
-		 * not be detructed before the nexthop entry is finished.
-		 * The second branch takes the reference in neith_create()
-		 */
-		n = neigh_entry->n;
-		neigh_clone(n);
+	}
+	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
+	if (!neigh_entry) {
+		neigh_release(n);
+		return -EINVAL;
 	}
 
 	/* If that is the first nexthop connected to that neigh, add to
@@ -1403,7 +1376,7 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 	if (list_empty(&nh->neigh_entry->nexthop_list))
 		list_del(&nh->neigh_entry->nexthop_neighs_list_node);
 
-	neigh_release(neigh_entry->n);
+	neigh_release(neigh_entry->key.n);
 }
 
 static struct mlxsw_sp_nexthop_group *
@@ -1463,11 +1436,11 @@ static bool mlxsw_sp_nexthop_match(struct mlxsw_sp_nexthop *nh,
 
 	for (i = 0; i < fi->fib_nhs; i++) {
 		struct fib_nh *fib_nh = &fi->fib_nh[i];
-		u32 gwip = ntohl(fib_nh->nh_gw);
+		struct neighbour *n = nh->neigh_entry->key.n;
 
-		if (memcmp(nh->neigh_entry->key.addr,
-			   &gwip, sizeof(u32)) == 0 &&
-		    nh->neigh_entry->key.dev == fib_nh->nh_dev)
+		if (memcmp(n->primary_key, &fib_nh->nh_gw,
+			   sizeof(fib_nh->nh_gw)) == 0 &&
+		    n->dev == fib_nh->nh_dev)
 			return true;
 	}
 	return false;
-- 
2.7.4

^ permalink raw reply related

* [patch net 0/2] mlxsw: Couple of router fixes
From: Jiri Pirko @ 2016-11-10 11:00 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz

From: Jiri Pirko <jiri@mellanox.com>

Jiri Pirko (2):
  mlxsw: spectrum_router: Fix handling of neighbour structure
  mlxsw: spectrum_router: Ignore FIB notification events for non-init
    namespaces

 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 98 ++++++++--------------
 1 file changed, 37 insertions(+), 61 deletions(-)

-- 
2.7.4

^ permalink raw reply

* Re: [Regression w/ patch] Restore network resistance to weird ICMP messages
From: Vicente Jiménez @ 2016-11-10 10:52 UTC (permalink / raw)
  To: David Miller
  Cc: Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, linux-kernel
In-Reply-To: <20161109.202210.1450575682716819324.davem@davemloft.net>

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

Corrected patch attached.
Thanks for the advices.
I was unaware of those style policies.

On Thu, Nov 10, 2016 at 2:22 AM, David Miller <davem@davemloft.net> wrote:
> From: Vicente Jiménez <googuy@gmail.com>
> Date: Mon, 7 Nov 2016 12:11:59 +0100
>
>> From bfc9a00e6b78d8eb60e46dacd7d761669d29a573 Mon Sep 17 00:00:00 2001
>> From: Vicente Jimenez Aguilar <googuy@gmail.com>
>> Date: Mon, 31 Oct 2016 13:10:29 +0100
>> Subject: [PATCH] ipv4: icmp: Fix pMTU handling for rarest case
>>
>> Restore network resistance to weird ICMP fragmentation needed messages
>> with next hop MTU equal to (or exceeding) dropped packet size
>>
>> Fixes: 46517008e116 ("ipv4: Kill ip_rt_frag_needed().")
>> Signed-off-by: Vicente Jimenez Aguilar <googuy@gmail.com>
>> ---
>>  net/ipv4/icmp.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> index 38abe70..c0af1d2 100644
>> --- a/net/ipv4/icmp.c
>> +++ b/net/ipv4/icmp.c
>> @@ -776,6 +776,7 @@ static bool icmp_unreach(struct sk_buff *skb)
>>       struct icmphdr *icmph;
>>       struct net *net;
>>       u32 info = 0;
>> +     unsigned short old_mtu;
>>
>>       net = dev_net(skb_dst(skb)->dev);
>>
>
> Order local variable declarations from longest to shortest line
> please.
>
>> +                             if ( info >= old_mtu )
>
> There should be no space after the '(' and before the ')' in this
> conditional.



-- 
saludos
vicente

[-- Attachment #2: 0001-ipv4-icmp-Fix-pMTU-handling-for-rarest-case.patch --]
[-- Type: text/x-patch, Size: 1273 bytes --]

From 88ac49fa287e2e0d3d3bc805dea1fec301aad1df Mon Sep 17 00:00:00 2001
From: Vicente Jimenez Aguilar <googuy@gmail.com>
Date: Mon, 31 Oct 2016 13:10:29 +0100
Subject: [PATCH] ipv4: icmp: Fix pMTU handling for rarest case

Restore network resistance to weird ICMP fragmentation needed messages
with next hop MTU equal to (or exceeding) dropped packet size

Fixes: 46517008e116 ("ipv4: Kill ip_rt_frag_needed().")
Signed-off-by: Vicente Jimenez Aguilar <googuy@gmail.com>
---
 net/ipv4/icmp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 38abe70..4c90d76 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -773,6 +773,7 @@ static bool icmp_tag_validation(int proto)
 static bool icmp_unreach(struct sk_buff *skb)
 {
 	const struct iphdr *iph;
+	unsigned short old_mtu;
 	struct icmphdr *icmph;
 	struct net *net;
 	u32 info = 0;
@@ -819,6 +820,12 @@ static bool icmp_unreach(struct sk_buff *skb)
 				/* fall through */
 			case 0:
 				info = ntohs(icmph->un.frag.mtu);
+				/* Handle weird case where next hop MTU is
+				 * equal to or exceeding dropped packet size
+				 */
+				old_mtu = ntohs(iph->tot_len);
+				if (info >= old_mtu)
+					info = old_mtu - 2;
 			}
 			break;
 		case ICMP_SR_FAILED:
-- 
2.7.3


^ permalink raw reply related

* Re: [net-next PATCH] amd-xgbe: use __maybe_unused to hide pm functions
From: Arnd Bergmann @ 2016-11-10 10:47 UTC (permalink / raw)
  To: David Miller
  Cc: thomas.lendacky, netdev, linux-kernel, Linux PM,
	Rafael J. Wysocki, Kirtika Ruchandani
In-Reply-To: <20161109.203251.2242759501432326877.davem@davemloft.net>

On Wednesday, November 9, 2016 8:32:51 PM CET David Miller wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue,  8 Nov 2016 14:37:32 +0100
> 
> > The amd-xgbe ethernet driver hides its suspend/resume functions
> > in #ifdef CONFIG_PM, but uses SIMPLE_DEV_PM_OPS() to make the
> > reference conditional on CONFIG_PM_SLEEP, which results in a
> > warning when PM_SLEEP is not set but PM is:
> > 
> > drivers/net/ethernet/amd/xgbe/xgbe-platform.c:553:12: error: 'xgbe_platform_resume' defined but not used [-Werror=unused-function]
> > drivers/net/ethernet/amd/xgbe/xgbe-platform.c:533:12: error: 'xgbe_platform_suspend' defined but not used [-Werror=unused-function]
> > 
> > This removes the incorrect #ifdef and instead uses a __maybe_unused
> > annotation to let the compiler know it can silently drop
> > the function definition.
> > 
> > Fixes: bd8255d8ba35 ("amd-xgbe: Prepare for supporting PCI devices")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > I originally submitted this when in March 2016, but the patch has not
> > yet made it upstream, and the file contents have moved around so
> > the old patch no longer applied so I'm resending the rebased version
> > now.
> 
> By and large, drivers handle this by using a CONFIG_PM_SLEEP ifdef.
> 
> Unless you can make an extremely convincing argument why not to do
> so here, I'd like you to handle it that way instead.

[adding linux-pm to Cc]

The main reason is that everyone gets the #ifdef wrong, I run into
half a dozen new build regressions with linux-next every week on
average, the typical problems being:

- testing CONFIG_PM_SLEEP instead of CONFIG_PM, leading to an unused
  function warning
- testing CONFIG_PM instead of CONFIG_PM_SLEEP, leading to a build
  failure
- calling a function outside of the #ifdef only from inside an
  otherwise correct #ifdef, again leading to an unused function
  warning
- causing a warning inside of the #ifdef but only testing if that
  is disabled, leading to a problem if the macro is set (this is
  rare these days for CONFIG_PM as that is normally enabled)

Using __maybe_unused avoids all of the above. My plan for the
long run however is to change the macro to something like

#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
        .suspend   = IS_ENABLED(CONFIG_PM_SLEEP) ? suspend_fn : NULL, \
        .resume    = IS_ENABLED(CONFIG_PM_SLEEP) ? resume_fn  : NULL, \
        .freeze    = IS_ENABLED(CONFIG_PM_SLEEP) ? suspend_fn : NULL, \
        .thaw      = IS_ENABLED(CONFIG_PM_SLEEP) ? resume_fn  : NULL, \
        .poweroff  = IS_ENABLED(CONFIG_PM_SLEEP) ? suspend_fn : NULL, \
        .restore   = IS_ENABLED(CONFIG_PM_SLEEP) ? resume_fn  : NULL, 

#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
        .runtime_suspend = IS_ENABLED(CONFIG_PM) ? suspend_fn : NULL, \
        .runtime_resume  = IS_ENABLED(CONFIG_PM) ? resume_fn  : NULL, \
        .runtime_idle    = IS_ENABLED(CONFIG_PM) ? idle_fn    : NULL,

I just haven't found the energy to start that discussion. With a definition
like this, we would need neither #ifdef nor __maybe_unused. Unfortunately
we can't just replace the existing macro while keeping the same name
because that would break every single user that has the #ifdef.

There was some discussion about that a while ago but no patch was merged
for it. I think in order to pull this off, we'd have to introduced
replacements for the existing six macros and change over the ~1000
existing users before removing the existing definitions:

   202  SET_SYSTEM_SLEEP_PM_OPS
    14  SET_LATE_SYSTEM_SLEEP_PM_OPS
    11  SET_NOIRQ_SYSTEM_SLEEP_PM_OPS
   218  SET_RUNTIME_PM_OPS
   551  SIMPLE_DEV_PM_OPS
    12  UNIVERSAL_DEV_PM_OPS

This would of course be a nontrivial amount of work, but it could
be mostly automated using coccinelle. In files per subsystem,
this would be

      7 drivers/acpi
     14 drivers/ata
     17 drivers/char
      6 drivers/crypto
     26 drivers/dma
      7 drivers/extcon
      7 drivers/gpio
     41 drivers/gpu
     10 drivers/hwmon
      7 drivers/hwtracing
     29 drivers/i2c
     90 drivers/iio
     37 drivers/media
     28 drivers/mfd
     15 drivers/misc
     52 drivers/mmc
     11 drivers/mtd
     67 drivers/net
     10 drivers/pinctrl
     19 drivers/platform
     13 drivers/power
      7 drivers/pwm
     44 drivers/rtc
     46 drivers/spi
     15 drivers/staging
     11 drivers/thermal
     22 drivers/tty
     37 drivers/usb
     32 drivers/video
     15 drivers/watchdog
     38 sound/pci
     52 sound/soc

	Arnd

^ permalink raw reply

* Re: [PATCH net-next v2 6/7] vxlan: simplify vxlan xmit
From: Jiri Benc @ 2016-11-10 10:08 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAOrHB_DneqHEg7Hi3SH0GebeN7TUoeKDevbwTatuhGgXrZOTbA@mail.gmail.com>

On Wed, 9 Nov 2016 19:35:00 -0800, Pravin Shelar wrote:
> On Wed, Nov 9, 2016 at 8:59 AM, Jiri Benc <jbenc@redhat.com> wrote:
> > Uninitialized old_iph.
> >
> It is initialized in begining of this function.

Right you are, sorry.

> TOS and TTL is initialized for LWT just else block. so I do not see
> any changes compared to current implementation.

Oh, right. We set them unconditionally from tunnel_info. I was
convinced we fallback to the interface config values when they're not
given and stopped reviewing the patch but we don't.

This is a nice cleanup, thanks!

Acked-by: Jiri Benc <jbenc@redhat.com>

^ permalink raw reply

* Re: [PATCH net-next v2 4/7] vxlan: improve vxlan route lookup checks.
From: Jiri Benc @ 2016-11-10  9:56 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAOrHB_BwcJ0K3YcqPrVkKxFjWX9DFT1NjS3BbrcNtKQ_p_y44w@mail.gmail.com>

On Wed, 9 Nov 2016 19:34:06 -0800, Pravin Shelar wrote:
> Why it would not help in non-ovs vxlan egress path? It avoids checking
> (if condition) for device loop.

I may be missing something but I count the same number of conditions
for each packet, they're just at a different place after the patch.

E.g. for IPv4, the "if (!sock4)" is moved from vxlan_xmit_one into
vxlan_get_route and the "rt" error handling stays logically the same
(three if conditions in the non-error path) but is moved into
vxlan_get_route. Similarly for IPv6.

 Jiri

^ permalink raw reply

* [PATCH net-next] ipv6: sr: fix IPv6 initialization failure without lwtunnels
From: David Lebrun @ 2016-11-10  9:55 UTC (permalink / raw)
  To: netdev; +Cc: lorenzo, davem, David Lebrun
In-Reply-To: <CAKD1Yr0fDFdDv9+S3i_rH6W6btEuvs9s-h33_N67iM4qrtDezQ@mail.gmail.com>

This patch compiles SR lwtunnels support only if CONFIG_LWTUNNEL=y.

If IPv6 is enabled and CONFIG_LWTUNNEL=n, then seg6_iptunnel_init()
fails with EOPNOTSUPP which in turn makes seg6_init() fail, blocking
the IPv6 initialization, with the following messages:

NET: Registered protocol family 10
IPv6: Attempt to unregister permanent protocol 6
IPv6: Attempt to unregister permanent protocol 136
IPv6: Attempt to unregister permanent protocol 17
NET: Unregistered protocol family 10

Fix commit 6c8702c60b88 ("ipv6: sr: add support for SRH encapsulation and injection with lwtunnels")

Tested with various combinations of CONFIG_IPV6 and CONFIG_LWTUNNEL.

Reported-by: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: David Lebrun <david.lebrun@uclouvain.be>
---
 net/ipv6/Kconfig  | 1 +
 net/ipv6/Makefile | 3 ++-
 net/ipv6/seg6.c   | 8 ++++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index 0f00811..030cdb6 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -292,6 +292,7 @@ config IPV6_PIMSM_V2
 config IPV6_SEG6_INLINE
 	bool "IPv6: direct Segment Routing Header insertion "
 	depends on IPV6
+	depends on LWTUNNEL
 	---help---
 	  Support for direct insertion of the Segment Routing Header,
 	  also known as inline mode. Be aware that direct insertion of
diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
index 129cad2..065c322 100644
--- a/net/ipv6/Makefile
+++ b/net/ipv6/Makefile
@@ -9,7 +9,7 @@ ipv6-objs :=	af_inet6.o anycast.o ip6_output.o ip6_input.o addrconf.o \
 		route.o ip6_fib.o ipv6_sockglue.o ndisc.o udp.o udplite.o \
 		raw.o icmp.o mcast.o reassembly.o tcp_ipv6.o ping.o \
 		exthdrs.o datagram.o ip6_flowlabel.o inet6_connection_sock.o \
-		udp_offload.o seg6.o seg6_iptunnel.o
+		udp_offload.o seg6.o
 
 ipv6-offload :=	ip6_offload.o tcpv6_offload.o exthdrs_offload.o
 
@@ -45,6 +45,7 @@ obj-$(CONFIG_IPV6_TUNNEL) += ip6_tunnel.o
 obj-$(CONFIG_IPV6_GRE) += ip6_gre.o
 obj-$(CONFIG_IPV6_FOU) += fou6.o
 obj-$(CONFIG_IPV6_SEG6_HMAC) += seg6_hmac.o
+obj-$(CONFIG_LWTUNNEL) += seg6_iptunnel.o
 
 obj-y += addrconf_core.o exthdrs_core.o ip6_checksum.o ip6_icmp.o
 obj-$(CONFIG_INET) += output_core.o protocol.o $(ipv6-offload)
diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
index 50f6e06..0f74f90 100644
--- a/net/ipv6/seg6.c
+++ b/net/ipv6/seg6.c
@@ -451,9 +451,11 @@ int __init seg6_init(void)
 	if (err)
 		goto out_unregister_genl;
 
+#ifdef CONFIG_LWTUNNEL
 	err = seg6_iptunnel_init();
 	if (err)
 		goto out_unregister_pernet;
+#endif
 
 #ifdef CONFIG_IPV6_SEG6_HMAC
 	err = seg6_hmac_init();
@@ -467,10 +469,14 @@ int __init seg6_init(void)
 	return err;
 #ifdef CONFIG_IPV6_SEG6_HMAC
 out_unregister_iptun:
+#ifdef CONFIG_LWTUNNEL
 	seg6_iptunnel_exit();
 #endif
+#endif
+#ifdef CONFIG_LWTUNNEL
 out_unregister_pernet:
 	unregister_pernet_subsys(&ip6_segments_ops);
+#endif
 out_unregister_genl:
 	genl_unregister_family(&seg6_genl_family);
 	goto out;
@@ -481,7 +487,9 @@ void seg6_exit(void)
 #ifdef CONFIG_IPV6_SEG6_HMAC
 	seg6_hmac_exit();
 #endif
+#ifdef CONFIG_LWTUNNEL
 	seg6_iptunnel_exit();
+#endif
 	unregister_pernet_subsys(&ip6_segments_ops);
 	genl_unregister_family(&seg6_genl_family);
 }
-- 
2.7.3

^ permalink raw reply related

* Re: [PATCH net-next v2 2/7] vxlan: simplify exception handling
From: Jiri Benc @ 2016-11-10  9:47 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAOrHB_AkVXDOaFKy_-ccRc--=wXG14TLfoC6Vc4HSBgjbDm3sA@mail.gmail.com>

On Wed, 9 Nov 2016 19:33:24 -0800, Pravin Shelar wrote:
> I have moved the dst error handling to vxlan_build_skb(), which is
> releasing the dst entry.

Hmm, right, I missed that. But it's weird. The previous logic was
confusing (skb is freed in case of error but not otherwise), I'm with
you on this change. But having the same confusing logic with dst_entry
instead doesn't improve things.

Could we free both skb and dst_entry from the caller?

Thanks,

 Jiri

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH] igb: use igb_adapter->io_addr instead of e1000_hw->hw_addr
From: Corinna Vinschen @ 2016-11-10  9:35 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Hisashi T Fujinaka, Cao jin, Netdev, intel-wired-lan,
	linux-kernel@vger.kernel.org, Izumi, Taku/泉 拓
In-Reply-To: <CAKgT0UeaoGfOOC1=h4pGk+=FDd5EqvgRGAdk1StLuiW1-M8tVA@mail.gmail.com>

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

On Nov  8 11:33, Alexander Duyck wrote:
> On Tue, Nov 8, 2016 at 10:37 AM, Corinna Vinschen <vinschen@redhat.com> wrote:
> > On Nov  8 09:16, Hisashi T Fujinaka wrote:
> >> On Tue, 8 Nov 2016, Corinna Vinschen wrote:
> >> > On Nov  8 15:06, Cao jin wrote:
> >> > > When running as guest, under certain condition, it will oops as following.
> >> > > writel() in igb_configure_tx_ring() results in oops, because hw->hw_addr
> >> > > is NULL. While other register access won't oops kernel because they use
> >> > > wr32/rd32 which have a defense against NULL pointer.
> >> > > [...]
> >> >
> >> > Incidentally we're just looking for a solution to that problem too.
> >> > Do three patches to fix the same problem at rougly the same time already
> >> > qualify as freak accident?
> >> >
> >> > FTR, I attached my current patch, which I was planning to submit after
> >> > some external testing.
> >> >
> >> > However, all three patches have one thing in common:  They workaround
> >> > a somewhat dubious resetting of the hardware address to NULL in case
> >> > reading from a register failed.
> >> >
> >> > That makes me wonder if setting the hardware address to NULL in
> >> > rd32/igb_rd32 is really such a good idea.  It's performed in a function
> >> > which return value is *never* tested for validity in the calling
> >> > functions and leads to subsequent crashes since no tests for hw_addr ==
> >> > NULL are performed.
> >> >
> >> > Maybe commit 22a8b2915 should be reconsidered?  Isn't there some more
> >> > graceful way to handle the "surprise removal"?
> >>
> >> Answering this from my home account because, well, work is Outlook.
> >>
> >> "Reconsidering" would be great. In fact, revert if if you'd like. I'm
> >> uncertain that the surprise removal code actually works the way I
> >> thought previously and I think I took a lot of it out of my local code.
> >>
> >> Unfortuantely I don't have any equipment that I can use to reproduce
> >> surprise removal any longer so that means I wouldn't be able to test
> >> anything. I have to defer to you or Cao Jin.
> >
> > I'm not too keen to rip out a PCIe NIC under power from my locale
> > desktop machine, but I think an actual surprise removal is not the
> > problem.
> >
> > As described in my git log entry, the error condition in igb_rd32 can be
> > triggered during a suspend.  The HW has been put into a sleep state but
> > some register read requests are apparently not guarded against that
> > situation.  Reading a register in this state returns -1, thus a suspend
> > is erroneously triggering the "surprise removal" sequence.
> 
> The question I would have is what is reading the device when it is in
> this state.  The watchdog and any other functions that would read the
> device should be disabled.
> 
> One possibility could be a race between a call to igb_close and the
> igb_suspend function.  We have seen some of those pop up recently on
> ixgbe and it looks like igb has the same bug.  We should probably be
> using the rtnl_lock to guarantee that netif_device_detach and the call
> to __igb_close are completed before igb_close could possibly be called
> by the network stack.

Do you have a pointer to the related ixgbe patch, by any chance?

> > Here's a raw idea:
> >
> > - Note that device is suspended in e1000_hw struct.  Don't trigger
> >   error sequence in igb_rd32 if so (...and return a 0 value???)
> 
> The thing is that a suspended device should not be accessed at all.
> If we are accessing it while it is suspended then that is a bug.  If
> you could throw a WARN_ON call in igb_rd32 to capture where this is
> being triggered that might be useful.
> 
> > - Otherwise assume it's actually a surprise removal.  In theory that
> >   should somehow trigger a device removal sequence, kind of like
> >   calling igb_remove, no?
> 
> Well a read of the MMIO region while suspended is more of a surprise
> read since there shouldn't be anything going on.  We need to isolate
> where that read is coming from and fix it.

That would be ideal, but the problem couldn't be reproduced yet apart
from at a customer's customer site.  It's not clear yet if we can access
the machine for further testing.


Corinna

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing
From: Lorenzo Colitti @ 2016-11-10  9:27 UTC (permalink / raw)
  To: David Lebrun; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <58243D57.2000805@uclouvain.be>

On Thu, Nov 10, 2016 at 6:26 PM, David Lebrun <david.lebrun@uclouvain.be> wrote:
> That would be easier indeed, but then this behavior should be documented
> somewhere: how the user would know that it has to enable CONFIG_LWTUNNEL ?

True.

I guess whether that question is important or not depends on how many
kernels enable CONFIG_LWTUNNEL. If pretty much all kernels already
enable it, then the point is moot.

^ permalink raw reply

* Re: [v16, 0/7] Fix eSDHC host version register bug
From: Geert Uytterhoeven @ 2016-11-10  9:26 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Yangbo Lu, linux-mmc, Scott Wood, Arnd Bergmann,
	linuxppc-dev@lists.ozlabs.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-clk, iommu,
	netdev@vger.kernel.org, Greg Kroah-Hartman, Mark Rutland,
	Rob Herring, Russell King, Jochen Friedrich, Joerg Roedel,
	Claudiu 
In-Reply-To: <CAPDyKFrcAN_pqgtGaUanfB2zh97zGcL23m5VDtJ3g==NJeRrfA@mail.gmail.com>

Hi Ulf,

On Wed, Nov 9, 2016 at 7:27 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 9 November 2016 at 04:14, Yangbo Lu <yangbo.lu@nxp.com> wrote:
>> This patchset is used to fix a host version register bug in the T4240-R1.0-R2.0
>> eSDHC controller. To match the SoC version and revision, 15 previous version
>> patchsets had tried many methods but all of them were rejected by reviewers.
>> Such as
>>         - dts compatible method
>>         - syscon method
>>         - ifdef PPC method
>>         - GUTS driver getting SVR method
>> Anrd suggested a soc_device_match method in v10, and this is the only available
>> method left now. This v11 patchset introduces the soc_device_match interface in
>> soc driver.
>>
>> The first four patches of Yangbo are to add the GUTS driver. This is used to
>> register a soc device which contain soc version and revision information.
>> The other three patches introduce the soc_device_match method in soc driver
>> and apply it on esdhc driver to fix this bug.
>>
>> ---
>> Changes for v15:
>>         - Dropped patch 'dt: bindings: update Freescale DCFG compatible'
>>           since the work had been done by below patch on ShawnGuo's linux tree.
>>           'dt-bindings: fsl: add LS1043A/LS1046A/LS2080A compatible for SCFG
>>            and DCFG'
>>         - Fixed error code issue in guts driver
>> Changes for v16:
>>         - Dropped patch 'powerpc/fsl: move mpc85xx.h to include/linux/fsl'
>>         - Added a bug-fix patch from Geert
>> ---
>>
>> Arnd Bergmann (1):
>>   base: soc: introduce soc_device_match() interface
>>
>> Geert Uytterhoeven (1):
>>   base: soc: Check for NULL SoC device attributes

> Thanks, applied on my mmc tree for next!

Oops, the above two commits (plus two more enhancements) are also a dependency
for Samsung and Renesas. Hence the plan was to use an immutable branch for
that...

Ulf, would it still be possible to replace these two commits in mmc/next:

    8b82c17a8ae533d6 base: soc: introduce soc_device_match() interface
    6fa350172b098f0f base: soc: Check for NULL SoC device attributes

by a merge of the immutable branch I've just created?
Cfr, the other thread  "[PATCH v2 0/7] soc: renesas: Identify SoC and register
with the SoC bus".

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing
From: David Lebrun @ 2016-11-10  9:26 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <CAKD1Yr1OGn_JS_REOL9g5gELf9s=VdGfebSrticF2=Vf-m5oKA@mail.gmail.com>

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

On 11/10/2016 10:14 AM, Lorenzo Colitti wrote:
> So you'd split SRH functionality in three parts? Core (default on),
> lwtunnel (default off) and inline?
> 
> It seems to me that once you've done the work to enable the core code
> to work when CONFIG_LWTUNNEL is off, you can just enable/disable the
> LWT part of SRH depending on whether CONFIG_LWTUNNEL is compiled in or
> not. That might save you a config option.

That would be easier indeed, but then this behavior should be documented
somewhere: how the user would know that it has to enable CONFIG_LWTUNNEL ?

David


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing
From: Lorenzo Colitti @ 2016-11-10  9:14 UTC (permalink / raw)
  To: David Lebrun; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <58243939.5000200@uclouvain.be>

On Thu, Nov 10, 2016 at 6:09 PM, David Lebrun <david.lebrun@uclouvain.be> wrote:
> Oops. The easiest fix I can think of is adding an option to
> enable/disable seg6's LWT which would select CONFIG_LWTUNNEL. That would
> still let the core SR processing be enabled by default.

So you'd split SRH functionality in three parts? Core (default on),
lwtunnel (default off) and inline?

It seems to me that once you've done the work to enable the core code
to work when CONFIG_LWTUNNEL is off, you can just enable/disable the
LWT part of SRH depending on whether CONFIG_LWTUNNEL is compiled in or
not. That might save you a config option.

^ permalink raw reply

* Re: [Xen PATCH] xen-netback: fix error handling output
From: Juergen Gross @ 2016-11-10  9:14 UTC (permalink / raw)
  To: Arnd Bergmann, David Vrabel
  Cc: Wei Liu, Paul Durrant, David S. Miller, Filipe Manco, xen-devel,
	netdev, linux-kernel
In-Reply-To: <20161108133449.1649345-1-arnd@arndb.de>

On 08/11/16 14:34, Arnd Bergmann wrote:
> The connect function prints an unintialized error code after an
> earlier initialization was removed:
> 
> drivers/net/xen-netback/xenbus.c: In function 'connect':
> drivers/net/xen-netback/xenbus.c:938:3: error: 'err' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This prints it as -EINVAL instead, which seems to be the most
> appropriate error code. Before the patch that caused the warning,
> this would print a positive number returned by vsscanf() instead,
> which is also wrong. We probably don't need a backport though,
> as fixing the warning here should be sufficient.
> 
> Fixes: f95842e7a9f2 ("xen: make use of xenbus_read_unsigned() in xen-netback")
> Fixes: 8d3d53b3e433 ("xen-netback: Add support for multiple queues")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied to xen/tip.git for-linus-4.10


Thanks,

Juergen

> ---
>  drivers/net/xen-netback/xenbus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 7356e00fac54..bfed79877b8a 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -935,7 +935,7 @@ static void connect(struct backend_info *be)
>  					"multi-queue-num-queues", 1);
>  	if (requested_num_queues > xenvif_max_queues) {
>  		/* buggy or malicious guest */
> -		xenbus_dev_fatal(dev, err,
> +		xenbus_dev_fatal(dev, -EINVAL,
>  				 "guest requested %u queues, exceeding the maximum of %u.",
>  				 requested_num_queues, xenvif_max_queues);
>  		return;
> 

^ permalink raw reply

* Re: [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing
From: David Lebrun @ 2016-11-10  9:09 UTC (permalink / raw)
  To: Lorenzo Colitti, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <CAKD1Yr0fDFdDv9+S3i_rH6W6btEuvs9s-h33_N67iM4qrtDezQ@mail.gmail.com>

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

On 11/10/2016 09:35 AM, Lorenzo Colitti wrote:
> Not sure how to fix this. The following makes IPv6 work again, but I
> suspect it mostly defeats the purpose of having SRH on by default:
> 
> +#ifdef CONFIG_LWTUNNEL
>          err = seg6_iptunnel_init();
>          if (err)
>                  goto out_unregister_pernet;
> +#endif
> 
> Enabling CONFIG_LWTUNNEL also makes IPv6 work again.
> 
> The breakage was caught by the kernel test robot, but the message was
> not particularly scary:

Oops. The easiest fix I can think of is adding an option to
enable/disable seg6's LWT which would select CONFIG_LWTUNNEL. That would
still let the core SR processing be enabled by default.

Comments ?

David


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing
From: Lorenzo Colitti @ 2016-11-10  8:35 UTC (permalink / raw)
  To: David Miller; +Cc: david.lebrun, netdev@vger.kernel.org
In-Reply-To: <20161109.204058.1740541278564630528.davem@davemloft.net>

On Thu, Nov 10, 2016 at 10:40 AM, David Miller <davem@davemloft.net> wrote:
>
> Series applied, but I wonder if using a Kconfig knob for the INLINE thing
> is overkill.

This breaks IPv6 completely if CONFIG_LWTUNNEL is disabled. When that
happens, seg6_iptunnel_init fails with EOPNOTSUPP and the stack fails
to initialize:

=====
NET: Registered protocol family 10
IPv6: Attempt to unregister permanent protocol 6
IPv6: Attempt to unregister permanent protocol 136
IPv6: Attempt to unregister permanent protocol 17
NET: Unregistered protocol family 10
=====

Not sure how to fix this. The following makes IPv6 work again, but I
suspect it mostly defeats the purpose of having SRH on by default:

+#ifdef CONFIG_LWTUNNEL
         err = seg6_iptunnel_init();
         if (err)
                 goto out_unregister_pernet;
+#endif

Enabling CONFIG_LWTUNNEL also makes IPv6 work again.

The breakage was caught by the kernel test robot, but the message was
not particularly scary:

http://marc.info/?l=linux-netdev&m=147848679207083&w=2

^ permalink raw reply

* Re: [PATCH net-next v5 0/9] net: add support for IPv6 Segment Routing
From: David Lebrun @ 2016-11-10  8:19 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20161109.204058.1740541278564630528.davem@davemloft.net>

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

On 11/10/2016 02:40 AM, David Miller wrote:
> Series applied, but I wonder if using a Kconfig knob for the INLINE thing
> is overkill.

Thanks :)

The Kconfig knob was a response to the concern that direct header
insertion breaks things. Moreover, if the IETF decides that direct
header insertion must be explicitely banned in rfc2460bis, then at least
we do not have a non-RFC compliant behavior by default.

David


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

^ permalink raw reply

* Re: Virtio_net support vxlan encapsulation package TSO offload discuss
From: Zhangming (James, Euler) @ 2016-11-10  6:19 UTC (permalink / raw)
  To: Jason Wang, netdev@vger.kernel.org; +Cc: Michael S. Tsirkin, Vlad Yasevic
In-Reply-To: <53d0322b-4ce1-d0db-4b53-f9e860ece2c5@redhat.com>

On 2016年11月09日 15:14, Jason Wang wrote:
>On 2016年11月08日 19:58, Zhangming (James, Euler) wrote:
>> On 2016年11月08日 19:17, Jason Wang wrote:
>>
>>> On 2016年11月08日 19:13, Jason Wang wrote:
>>>> Cc Michael
>>>>
>>>> On 2016年11月08日 16:34, Zhangming (James, Euler) wrote:
>>>>> In container scenario, OVS is installed in the Virtual machine, and 
>>>>> all the containers connected to the OVS will communicated through 
>>>>> VXLAN encapsulation.
>>>>>
>>>>> By now, virtio_net does not support TSO offload for VXLAN 
>>>>> encapsulated TSO package. In this condition, the performance is not 
>>>>> good, sender is bottleneck
>>>>>
>>>>> I googled this scenario, but I didn’t find any information. Will 
>>>>> virtio_net support VXLAN encapsulation package TSO offload later?
>>>>>
>>>> Yes and for both sender and receiver.
>>>>
>>>>> My idea is virtio_net open encapsulated TSO offload, and transport 
>>>>> encapsulation info to TUN, TUN will parse the info and build skb 
>>>>> with encapsulation info.
>>>>>
>>>>> OVS or kernel on the host should be modified to support this. Using 
>>>>> this method, the TCP performance aremore than 2x as before.
>>>>>
>>>>> Any advice and suggestions for this idea or new idea will be 
>>>>> greatly appreciated!
>>>>>
>>>>> Best regards,
>>>>>
>>>>>     James zhang
>>>>>
>>>> Sounds very good. And we may also need features bits
>>>> (VIRTIO_NET_F_GUEST|HOST_GSO_X) for this.
>>>>
>>>> This is in fact one of items in networking todo list. (See 
>>>> http://www.linux-kvm.org/page/NetworkingTodo). While at it, we'd 
>>>> better support not only VXLAN but also other tunnels.
>>> Cc Vlad who is working on extending virtio-net headers.
>>>
>>>> We can start with the spec work, or if you've already had some bits 
>>>> you can post them as RFC for early review.
>>>>
>>>> Thanks
>> Below is my demo code
>> Virtio_net.c
>> static int virtnet_probe(struct virtio_device *vdev), add belows codes:
>>          if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||				// avoid gso segment, it should be negotiation later, because in the demo I reuse num_buffers.
>>              virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>>                  dev->hw_enc_features |= NETIF_F_TSO;
>>                  dev->hw_enc_features |= NETIF_F_ALL_CSUM;
>>                  dev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL;
>>                  dev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
>>                  dev->hw_enc_features |= NETIF_F_GSO_TUNNEL_REMCSUM;
>>
>>                  dev->features |= NETIF_F_GSO_UDP_TUNNEL;
>>                  dev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
>>                  dev->features |= NETIF_F_GSO_TUNNEL_REMCSUM;
>>          }
>>
>> static int xmit_skb(struct send_queue *sq, struct sk_buff *skb), add 
>> below to pieces of codes
>>
>>                  if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL)
>>                          hdr->hdr.gso_type |= VIRTIO_NET_HDR_GSO_TUNNEL;
>>                  if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM)
>>                          hdr->hdr.gso_type |= VIRTIO_NET_HDR_GSO_TUNNEL_CSUM;
>>                  if (skb_shinfo(skb)->gso_type & SKB_GSO_TUNNEL_REMCSUM)
>>                          hdr->hdr.gso_type |= 
>> VIRTIO_NET_HDR_GSO_TUNNEL_REMCSUM;
>>
>>          if (skb->encapsulation && skb_is_gso(skb)) {
>>                  inner_mac_len = skb_inner_network_header(skb) - skb_inner_mac_header(skb);
>>                  tnl_len = skb_inner_mac_header(skb) - skb_mac_header(skb);
>>                  if ( !(inner_mac_len >> DATA_LEN_SHIFT) && !(tnl_len >> DATA_LEN_SHIFT) ) {
>>                          hdr->hdr.flags |= VIRTIO_NET_HDR_F_ENCAPSULATION;
>>                          hdr->num_buffers = (__virtio16)((inner_mac_len << DATA_LEN_SHIFT) | tnl_len);		//we reuse num_buffers for simple , we should add extend member for later.
>>                  }  else
>>                          hdr->num_buffers = 0;
>>          }
>>
>> Tun.c
>>                  if (memcpy_fromiovecend((void *)&hdr, iv, offset, tun->vnet_hdr_sz))		//read header with negotiation length
>>                          return -EFAULT;
>>
>>                  if (hdr.gso_type & VIRTIO_NET_HDR_GSO_TUNNEL)					//set tunnel gso info
>>                          skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
>>                  if (hdr.gso_type & VIRTIO_NET_HDR_GSO_TUNNEL_CSUM)
>>                          skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL_CSUM;
>>                  if (hdr.gso_type & VIRTIO_NET_HDR_GSO_TUNNEL_REMCSUM)
>>                          skb_shinfo(skb)->gso_type |= 
>> SKB_GSO_TUNNEL_REMCSUM;
>>
>>          if (hdr.flags & VIRTIO_NET_HDR_F_ENCAPSULATION) {						//read tunnel info from header and set to built skb.
>>                  tnl_len = tun16_to_cpu(tun, hdr.num_buffers) & TUN_TNL_LEN_MASK;
>>                  payload_mac_len = tun16_to_cpu(tun, hdr.num_buffers) >> TUN_DATA_LEN_SHIFT;
>>                  mac_len = skb_network_header(skb) - skb_mac_header(skb);
>>                  skb_set_inner_mac_header(skb, tnl_len - mac_len);
>>                  skb_set_inner_network_header(skb, tnl_len + payload_mac_len - mac_len);
>>                  skb->encapsulation = 1;
>>          }
>>
>>
>
>Something like this, and you probably need do something more:
>
>- use net-next.git to generate the patch (for the latest code)
>- add feature negotiation
>- tun/macvtap/qemu patches for this, you can start with tun/macvtap patches
>- support for all other SKB_GSO_* types which is not supported
>- use a new field instead of num_buffers
>- a virtio spec patch to describe the support for encapsulation offload
>
>Thanks

Thank you for your advice, I will start it right now.

Thanks


^ 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