[PATCH V2 1/4] net-next: dsa: move struct dsa_device_ops to the global header file

[PATCH V2 1/4] net-next: dsa: move struct dsa_device_ops to the global header file

[John Crispin]

We need to access this struct from within the flow_dissector to fix
dissection for packets coming in on DSA devices.

Signed-off-by: John Crispin <john@phrozen.org>
---
 include/net/dsa.h  | 7 +++++++
 net/dsa/dsa_priv.h | 7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 88da272d20d0..a4c0d52abc80 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -101,6 +101,13 @@ struct dsa_platform_data {
 
 struct packet_type;
 
+struct dsa_device_ops {
+	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
+	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
+			       struct packet_type *pt,
+			       struct net_device *orig_dev);
+};
+
 struct dsa_switch_tree {
 	struct list_head	list;
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 55982cc39b24..62ea3663c2c6 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -65,13 +65,6 @@ struct dsa_notifier_vlan_info {
 	int port;
 };
 
-struct dsa_device_ops {
-	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
-	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
-			       struct packet_type *pt,
-			       struct net_device *orig_dev);
-};
-
 struct dsa_slave_priv {
 	/* Copy of dp->ds->dst->tag_ops->xmit for faster access in hot path */
 	struct sk_buff *	(*xmit)(struct sk_buff *skb,
-- 
2.11.0

[PATCH V2 2/4] net-next: dsa: add 802.3 protocol offset to struct dsa_device_ops

[John Crispin]

Adding these 2 new fields allows a DSA device to indicate the offsets of
the 802.3 header caused by the insertion of the switches tag.

Signed-off-by: John Crispin <john@phrozen.org>
---
 include/net/dsa.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index a4c0d52abc80..b98bc3621905 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -106,6 +106,11 @@ struct dsa_device_ops {
 	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
 			       struct packet_type *pt,
 			       struct net_device *orig_dev);
+	/*
+	 * Network header and 802.3 protocol offsets
+	 */
+	int hash_nh_off;
+	int hash_proto_off;
 };
 
 struct dsa_switch_tree {
-- 
2.11.0

[PATCH V2 3/4] net-next: dsa: fix flow dissection

[John Crispin]

RPS and probably other kernel features are currently broken on some if not
all DSA devices. The root cause of this is that skb_hash will call the
flow_dissector. At this point the skb still contains the magic switch header
and the skb->protocol field is not set up to the correct 802.3 value yet.
By the time the tag specific code is called, removing the header and
properly setting the protocol an invalid hash is already set. In the case
of the mt7530 this will result in all flows always having the same hash.

This patch makes the flow dissector honour the nh and protocol offset
defined by the dsa tag driver thus fixing dissection, hashing and RPS.

Signed-off-by: John Crispin <john@phrozen.org>
---
 net/core/flow_dissector.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index fc5fc4594c90..1268ae75c3b3 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -4,6 +4,7 @@
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <linux/if_vlan.h>
+#include <net/dsa.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/gre.h>
@@ -440,6 +441,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			 skb->vlan_proto : skb->protocol;
 		nhoff = skb_network_offset(skb);
 		hlen = skb_headlen(skb);
+
+		if (unlikely(netdev_uses_dsa(skb->dev))) {
+			const struct dsa_device_ops *ops;
+			u8 *p = (u8 *)data;
+
+			ops = skb->dev->dsa_ptr->tag_ops;
+			if (ops->hash_proto_off)
+				proto = (u16)p[ops->hash_proto_off];
+			hlen -= ops->hash_nh_off;
+			nhoff += ops->hash_nh_off;
+		}
 	}
 
 	/* It is ensured by skb_flow_dissector_init() that control key will
-- 
2.11.0

[PATCH V2 4/4] net-next: tag_mtk: add nh and proto offsets to the ops struct

[John Crispin]

The MT7530 inserts the 4 magic header in between the 802.3 address and
protocol field. The patch defines these header such that the flow_disector
can properly parse the packet and thus allows hashing to function properly.

Signed-off-by: John Crispin <john@phrozen.org>
---
 net/dsa/tag_mtk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index 2f32b7ea3365..142322988202 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -88,6 +88,8 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 }
 
 const struct dsa_device_ops mtk_netdev_ops = {
-	.xmit	= mtk_tag_xmit,
-	.rcv	= mtk_tag_rcv,
+	.xmit		= mtk_tag_xmit,
+	.rcv		= mtk_tag_rcv,
+	.hash_nh_off	= 4,
+	.hash_proto_off	= 2,
 };
-- 
2.11.0

Re: [PATCH V2 1/4] net-next: dsa: move struct dsa_device_ops to the global header file

[David Miller]

A proper patch series submission must always have a proper
"[PATCH 0/N] ..." header posting that explains at a high level
what the patch series does, how it is doing it, and why it is
doing it that way.

Thank you.

Re: [PATCH V2 3/4] net-next: dsa: fix flow dissection

[kbuild test robot]

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

Hi John,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.13-rc1 next-20170721]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/John-Crispin/net-next-dsa-move-struct-dsa_device_ops-to-the-global-header-file/20170724-034620
config: i386-randconfig-a0-07231650 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net/core/flow_dissector.c: In function '__skb_flow_dissect':
>> net/core/flow_dissector.c:449:18: error: 'struct net_device' has no member named 'dsa_ptr'
       ops = skb->dev->dsa_ptr->tag_ops;
                     ^

vim +449 net/core/flow_dissector.c

   404	
   405	/**
   406	 * __skb_flow_dissect - extract the flow_keys struct and return it
   407	 * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
   408	 * @flow_dissector: list of keys to dissect
   409	 * @target_container: target structure to put dissected values into
   410	 * @data: raw buffer pointer to the packet, if NULL use skb->data
   411	 * @proto: protocol for which to get the flow, if @data is NULL use skb->protocol
   412	 * @nhoff: network header offset, if @data is NULL use skb_network_offset(skb)
   413	 * @hlen: packet header length, if @data is NULL use skb_headlen(skb)
   414	 *
   415	 * The function will try to retrieve individual keys into target specified
   416	 * by flow_dissector from either the skbuff or a raw buffer specified by the
   417	 * rest parameters.
   418	 *
   419	 * Caller must take care of zeroing target container memory.
   420	 */
   421	bool __skb_flow_dissect(const struct sk_buff *skb,
   422				struct flow_dissector *flow_dissector,
   423				void *target_container,
   424				void *data, __be16 proto, int nhoff, int hlen,
   425				unsigned int flags)
   426	{
   427		struct flow_dissector_key_control *key_control;
   428		struct flow_dissector_key_basic *key_basic;
   429		struct flow_dissector_key_addrs *key_addrs;
   430		struct flow_dissector_key_ports *key_ports;
   431		struct flow_dissector_key_icmp *key_icmp;
   432		struct flow_dissector_key_tags *key_tags;
   433		struct flow_dissector_key_vlan *key_vlan;
   434		bool skip_vlan = false;
   435		u8 ip_proto = 0;
   436		bool ret;
   437	
   438		if (!data) {
   439			data = skb->data;
   440			proto = skb_vlan_tag_present(skb) ?
   441				 skb->vlan_proto : skb->protocol;
   442			nhoff = skb_network_offset(skb);
   443			hlen = skb_headlen(skb);
   444	
   445			if (unlikely(netdev_uses_dsa(skb->dev))) {
   446				const struct dsa_device_ops *ops;
   447				u8 *p = (u8 *)data;
   448	
 > 449				ops = skb->dev->dsa_ptr->tag_ops;
   450				if (ops->hash_proto_off)
   451					proto = (u16)p[ops->hash_proto_off];
   452				hlen -= ops->hash_nh_off;
   453				nhoff += ops->hash_nh_off;
   454			}
   455		}
   456	
   457		/* It is ensured by skb_flow_dissector_init() that control key will
   458		 * be always present.
   459		 */
   460		key_control = skb_flow_dissector_target(flow_dissector,
   461							FLOW_DISSECTOR_KEY_CONTROL,
   462							target_container);
   463	
   464		/* It is ensured by skb_flow_dissector_init() that basic key will
   465		 * be always present.
   466		 */
   467		key_basic = skb_flow_dissector_target(flow_dissector,
   468						      FLOW_DISSECTOR_KEY_BASIC,
   469						      target_container);
   470	
   471		if (dissector_uses_key(flow_dissector,
   472				       FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
   473			struct ethhdr *eth = eth_hdr(skb);
   474			struct flow_dissector_key_eth_addrs *key_eth_addrs;
   475	
   476			key_eth_addrs = skb_flow_dissector_target(flow_dissector,
   477								  FLOW_DISSECTOR_KEY_ETH_ADDRS,
   478								  target_container);
   479			memcpy(key_eth_addrs, &eth->h_dest, sizeof(*key_eth_addrs));
   480		}
   481	
   482	proto_again:
   483		switch (proto) {
   484		case htons(ETH_P_IP): {
   485			const struct iphdr *iph;
   486			struct iphdr _iph;
   487	ip:
   488			iph = __skb_header_pointer(skb, nhoff, sizeof(_iph), data, hlen, &_iph);
   489			if (!iph || iph->ihl < 5)
   490				goto out_bad;
   491			nhoff += iph->ihl * 4;
   492	
   493			ip_proto = iph->protocol;
   494	
   495			if (dissector_uses_key(flow_dissector,
   496					       FLOW_DISSECTOR_KEY_IPV4_ADDRS)) {
   497				key_addrs = skb_flow_dissector_target(flow_dissector,
   498								      FLOW_DISSECTOR_KEY_IPV4_ADDRS,
   499								      target_container);
   500	
   501				memcpy(&key_addrs->v4addrs, &iph->saddr,
   502				       sizeof(key_addrs->v4addrs));
   503				key_control->addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
   504			}
   505	
   506			if (ip_is_fragment(iph)) {
   507				key_control->flags |= FLOW_DIS_IS_FRAGMENT;
   508	
   509				if (iph->frag_off & htons(IP_OFFSET)) {
   510					goto out_good;
   511				} else {
   512					key_control->flags |= FLOW_DIS_FIRST_FRAG;
   513					if (!(flags & FLOW_DISSECTOR_F_PARSE_1ST_FRAG))
   514						goto out_good;
   515				}
   516			}
   517	
   518			__skb_flow_dissect_ipv4(skb, flow_dissector,
   519						target_container, data, iph);
   520	
   521			if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
   522				goto out_good;
   523	
   524			break;
   525		}
   526		case htons(ETH_P_IPV6): {
   527			const struct ipv6hdr *iph;
   528			struct ipv6hdr _iph;
   529	
   530	ipv6:
   531			iph = __skb_header_pointer(skb, nhoff, sizeof(_iph), data, hlen, &_iph);
   532			if (!iph)
   533				goto out_bad;
   534	
   535			ip_proto = iph->nexthdr;
   536			nhoff += sizeof(struct ipv6hdr);
   537	
   538			if (dissector_uses_key(flow_dissector,
   539					       FLOW_DISSECTOR_KEY_IPV6_ADDRS)) {
   540				key_addrs = skb_flow_dissector_target(flow_dissector,
   541								      FLOW_DISSECTOR_KEY_IPV6_ADDRS,
   542								      target_container);
   543	
   544				memcpy(&key_addrs->v6addrs, &iph->saddr,
   545				       sizeof(key_addrs->v6addrs));
   546				key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
   547			}
   548	
   549			if ((dissector_uses_key(flow_dissector,
   550						FLOW_DISSECTOR_KEY_FLOW_LABEL) ||
   551			     (flags & FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL)) &&
   552			    ip6_flowlabel(iph)) {
   553				__be32 flow_label = ip6_flowlabel(iph);
   554	
   555				if (dissector_uses_key(flow_dissector,
   556						       FLOW_DISSECTOR_KEY_FLOW_LABEL)) {
   557					key_tags = skb_flow_dissector_target(flow_dissector,
   558									     FLOW_DISSECTOR_KEY_FLOW_LABEL,
   559									     target_container);
   560					key_tags->flow_label = ntohl(flow_label);
   561				}
   562				if (flags & FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL)
   563					goto out_good;
   564			}
   565	
   566			__skb_flow_dissect_ipv6(skb, flow_dissector,
   567						target_container, data, iph);
   568	
   569			if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
   570				goto out_good;
   571	
   572			break;
   573		}
   574		case htons(ETH_P_8021AD):
   575		case htons(ETH_P_8021Q): {
   576			const struct vlan_hdr *vlan;
   577			struct vlan_hdr _vlan;
   578			bool vlan_tag_present = skb && skb_vlan_tag_present(skb);
   579	
   580			if (vlan_tag_present)
   581				proto = skb->protocol;
   582	
   583			if (!vlan_tag_present || eth_type_vlan(skb->protocol)) {
   584				vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
   585							    data, hlen, &_vlan);
   586				if (!vlan)
   587					goto out_bad;
   588				proto = vlan->h_vlan_encapsulated_proto;
   589				nhoff += sizeof(*vlan);
   590				if (skip_vlan)
   591					goto proto_again;
   592			}
   593	
   594			skip_vlan = true;
   595			if (dissector_uses_key(flow_dissector,
   596					       FLOW_DISSECTOR_KEY_VLAN)) {
   597				key_vlan = skb_flow_dissector_target(flow_dissector,
   598								     FLOW_DISSECTOR_KEY_VLAN,
   599								     target_container);
   600	
   601				if (vlan_tag_present) {
   602					key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
   603					key_vlan->vlan_priority =
   604						(skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
   605				} else {
   606					key_vlan->vlan_id = ntohs(vlan->h_vlan_TCI) &
   607						VLAN_VID_MASK;
   608					key_vlan->vlan_priority =
   609						(ntohs(vlan->h_vlan_TCI) &
   610						 VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
   611				}
   612			}
   613	
   614			goto proto_again;
   615		}
   616		case htons(ETH_P_PPP_SES): {
   617			struct {
   618				struct pppoe_hdr hdr;
   619				__be16 proto;
   620			} *hdr, _hdr;
   621			hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
   622			if (!hdr)
   623				goto out_bad;
   624			proto = hdr->proto;
   625			nhoff += PPPOE_SES_HLEN;
   626			switch (proto) {
   627			case htons(PPP_IP):
   628				goto ip;
   629			case htons(PPP_IPV6):
   630				goto ipv6;
   631			default:
   632				goto out_bad;
   633			}
   634		}
   635		case htons(ETH_P_TIPC): {
   636			struct {
   637				__be32 pre[3];
   638				__be32 srcnode;
   639			} *hdr, _hdr;
   640			hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
   641			if (!hdr)
   642				goto out_bad;
   643	
   644			if (dissector_uses_key(flow_dissector,
   645					       FLOW_DISSECTOR_KEY_TIPC_ADDRS)) {
   646				key_addrs = skb_flow_dissector_target(flow_dissector,
   647								      FLOW_DISSECTOR_KEY_TIPC_ADDRS,
   648								      target_container);
   649				key_addrs->tipcaddrs.srcnode = hdr->srcnode;
   650				key_control->addr_type = FLOW_DISSECTOR_KEY_TIPC_ADDRS;
   651			}
   652			goto out_good;
   653		}
   654	
   655		case htons(ETH_P_MPLS_UC):
   656		case htons(ETH_P_MPLS_MC):
   657	mpls:
   658			switch (__skb_flow_dissect_mpls(skb, flow_dissector,
   659							target_container, data,
   660							nhoff, hlen)) {
   661			case FLOW_DISSECT_RET_OUT_GOOD:
   662				goto out_good;
   663			case FLOW_DISSECT_RET_OUT_BAD:
   664			default:
   665				goto out_bad;
   666			}
   667		case htons(ETH_P_FCOE):
   668			if ((hlen - nhoff) < FCOE_HEADER_LEN)
   669				goto out_bad;
   670	
   671			nhoff += FCOE_HEADER_LEN;
   672			goto out_good;
   673	
   674		case htons(ETH_P_ARP):
   675		case htons(ETH_P_RARP):
   676			switch (__skb_flow_dissect_arp(skb, flow_dissector,
   677						       target_container, data,
   678						       nhoff, hlen)) {
   679			case FLOW_DISSECT_RET_OUT_GOOD:
   680				goto out_good;
   681			case FLOW_DISSECT_RET_OUT_BAD:
   682			default:
   683				goto out_bad;
   684			}
   685		default:
   686			goto out_bad;
   687		}
   688	
   689	ip_proto_again:
   690		switch (ip_proto) {
   691		case IPPROTO_GRE:
   692			switch (__skb_flow_dissect_gre(skb, key_control, flow_dissector,
   693						       target_container, data,
   694						       &proto, &nhoff, &hlen, flags)) {
   695			case FLOW_DISSECT_RET_OUT_GOOD:
   696				goto out_good;
   697			case FLOW_DISSECT_RET_OUT_BAD:
   698				goto out_bad;
   699			case FLOW_DISSECT_RET_OUT_PROTO_AGAIN:
   700				goto proto_again;
   701			}
   702		case NEXTHDR_HOP:
   703		case NEXTHDR_ROUTING:
   704		case NEXTHDR_DEST: {
   705			u8 _opthdr[2], *opthdr;
   706	
   707			if (proto != htons(ETH_P_IPV6))
   708				break;
   709	
   710			opthdr = __skb_header_pointer(skb, nhoff, sizeof(_opthdr),
   711						      data, hlen, &_opthdr);
   712			if (!opthdr)
   713				goto out_bad;
   714	
   715			ip_proto = opthdr[0];
   716			nhoff += (opthdr[1] + 1) << 3;
   717	
   718			goto ip_proto_again;
   719		}
   720		case NEXTHDR_FRAGMENT: {
   721			struct frag_hdr _fh, *fh;
   722	
   723			if (proto != htons(ETH_P_IPV6))
   724				break;
   725	
   726			fh = __skb_header_pointer(skb, nhoff, sizeof(_fh),
   727						  data, hlen, &_fh);
   728	
   729			if (!fh)
   730				goto out_bad;
   731	
   732			key_control->flags |= FLOW_DIS_IS_FRAGMENT;
   733	
   734			nhoff += sizeof(_fh);
   735			ip_proto = fh->nexthdr;
   736	
   737			if (!(fh->frag_off & htons(IP6_OFFSET))) {
   738				key_control->flags |= FLOW_DIS_FIRST_FRAG;
   739				if (flags & FLOW_DISSECTOR_F_PARSE_1ST_FRAG)
   740					goto ip_proto_again;
   741			}
   742			goto out_good;
   743		}
   744		case IPPROTO_IPIP:
   745			proto = htons(ETH_P_IP);
   746	
   747			key_control->flags |= FLOW_DIS_ENCAPSULATION;
   748			if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
   749				goto out_good;
   750	
   751			goto ip;
   752		case IPPROTO_IPV6:
   753			proto = htons(ETH_P_IPV6);
   754	
   755			key_control->flags |= FLOW_DIS_ENCAPSULATION;
   756			if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
   757				goto out_good;
   758	
   759			goto ipv6;
   760		case IPPROTO_MPLS:
   761			proto = htons(ETH_P_MPLS_UC);
   762			goto mpls;
   763		case IPPROTO_TCP:
   764			__skb_flow_dissect_tcp(skb, flow_dissector, target_container,
   765					       data, nhoff, hlen);
   766			break;
   767		default:
   768			break;
   769		}
   770	
   771		if (dissector_uses_key(flow_dissector,
   772				       FLOW_DISSECTOR_KEY_PORTS)) {
   773			key_ports = skb_flow_dissector_target(flow_dissector,
   774							      FLOW_DISSECTOR_KEY_PORTS,
   775							      target_container);
   776			key_ports->ports = __skb_flow_get_ports(skb, nhoff, ip_proto,
   777								data, hlen);
   778		}
   779	
   780		if (dissector_uses_key(flow_dissector,
   781				       FLOW_DISSECTOR_KEY_ICMP)) {
   782			key_icmp = skb_flow_dissector_target(flow_dissector,
   783							     FLOW_DISSECTOR_KEY_ICMP,
   784							     target_container);
   785			key_icmp->icmp = skb_flow_get_be16(skb, nhoff, data, hlen);
   786		}
   787	
   788	out_good:
   789		ret = true;
   790	
   791		key_control->thoff = (u16)nhoff;
   792	out:
   793		key_basic->n_proto = proto;
   794		key_basic->ip_proto = ip_proto;
   795	
   796		return ret;
   797	
   798	out_bad:
   799		ret = false;
   800		key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
   801		goto out;
   802	}
   803	EXPORT_SYMBOL(__skb_flow_dissect);
   804	

---
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: 24995 bytes --]

Re: [PATCH V2 1/4] net-next: dsa: move struct dsa_device_ops to the global header file

[Andrew Lunn]

On Fri, Jul 21, 2017 at 10:58:10AM +0200, John Crispin wrote:
> We need to access this struct from within the flow_dissector to fix
> dissection for packets coming in on DSA devices.
> 
> Signed-off-by: John Crispin <john@phrozen.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V2 3/4] net-next: dsa: fix flow dissection

[Andrew Lunn]

On Fri, Jul 21, 2017 at 10:58:12AM +0200, John Crispin wrote:
> RPS and probably other kernel features are currently broken on some if not
> all DSA devices. The root cause of this is that skb_hash will call the
> flow_dissector. At this point the skb still contains the magic switch header
> and the skb->protocol field is not set up to the correct 802.3 value yet.
> By the time the tag specific code is called, removing the header and
> properly setting the protocol an invalid hash is already set. In the case
> of the mt7530 this will result in all flows always having the same hash.
> 
> This patch makes the flow dissector honour the nh and protocol offset
> defined by the dsa tag driver thus fixing dissection, hashing and RPS.
> 
> Signed-off-by: John Crispin <john@phrozen.org>
> ---
>  net/core/flow_dissector.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index fc5fc4594c90..1268ae75c3b3 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -4,6 +4,7 @@
>  #include <linux/ip.h>
>  #include <linux/ipv6.h>
>  #include <linux/if_vlan.h>
> +#include <net/dsa.h>
>  #include <net/ip.h>
>  #include <net/ipv6.h>
>  #include <net/gre.h>
> @@ -440,6 +441,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>  			 skb->vlan_proto : skb->protocol;
>  		nhoff = skb_network_offset(skb);
>  		hlen = skb_headlen(skb);
> +
> +		if (unlikely(netdev_uses_dsa(skb->dev))) {
> +			const struct dsa_device_ops *ops;
> +			u8 *p = (u8 *)data;
> +
> +			ops = skb->dev->dsa_ptr->tag_ops;
> +			if (ops->hash_proto_off)
> +				proto = (u16)p[ops->hash_proto_off];

Hi John

Unfortunately, this is not generic enough to work for DSA and EDSA
tagging. With these tagging protocols, the size of the tag depends on
the presence or not of a VLAN header.

To make this work for all tagging protocols, we are going to need to
add an a new op to tag_ops.

    Andrew

Re: [PATCH V2 3/4] net-next: dsa: fix flow dissection

[John Crispin]

On 26/07/17 17:10, Andrew Lunn wrote:
> On Fri, Jul 21, 2017 at 10:58:12AM +0200, John Crispin wrote:
>> RPS and probably other kernel features are currently broken on some if not
>> all DSA devices. The root cause of this is that skb_hash will call the
>> flow_dissector. At this point the skb still contains the magic switch header
>> and the skb->protocol field is not set up to the correct 802.3 value yet.
>> By the time the tag specific code is called, removing the header and
>> properly setting the protocol an invalid hash is already set. In the case
>> of the mt7530 this will result in all flows always having the same hash.
>>
>> This patch makes the flow dissector honour the nh and protocol offset
>> defined by the dsa tag driver thus fixing dissection, hashing and RPS.
>>
>> Signed-off-by: John Crispin <john@phrozen.org>
>> ---
>>   net/core/flow_dissector.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index fc5fc4594c90..1268ae75c3b3 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -4,6 +4,7 @@
>>   #include <linux/ip.h>
>>   #include <linux/ipv6.h>
>>   #include <linux/if_vlan.h>
>> +#include <net/dsa.h>
>>   #include <net/ip.h>
>>   #include <net/ipv6.h>
>>   #include <net/gre.h>
>> @@ -440,6 +441,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>   			 skb->vlan_proto : skb->protocol;
>>   		nhoff = skb_network_offset(skb);
>>   		hlen = skb_headlen(skb);
>> +
>> +		if (unlikely(netdev_uses_dsa(skb->dev))) {
>> +			const struct dsa_device_ops *ops;
>> +			u8 *p = (u8 *)data;
>> +
>> +			ops = skb->dev->dsa_ptr->tag_ops;
>> +			if (ops->hash_proto_off)
>> +				proto = (u16)p[ops->hash_proto_off];
> Hi John
>
> Unfortunately, this is not generic enough to work for DSA and EDSA
> tagging. With these tagging protocols, the size of the tag depends on
> the presence or not of a VLAN header.
>
> To make this work for all tagging protocols, we are going to need to
> add an a new op to tag_ops.
>
>      Andrew
Hi Andrew,

thanks for the feedback. should I add 2 callbacks for each of the 2 
parameters ?

     John

Re: [PATCH V2 3/4] net-next: dsa: fix flow dissection

[Andrew Lunn]

> thanks for the feedback. should I add 2 callbacks for each of the 2
> parameters ?

Hi John

A single callback is better. We don't want to have to peek into the
packet twice to determine two values.

       Andrew