2017-07-21 08:58:58

by John Crispin

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

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 <[email protected]>
---
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


2017-07-21 08:58:59

by John Crispin

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

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 <[email protected]>
---
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

2017-07-21 08:59:02

by John Crispin

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

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 <[email protected]>
---
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

2017-07-21 08:59:15

by John Crispin

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

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 <[email protected]>
---
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

2017-07-21 19:12:53

by David Miller

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


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.

2017-07-23 20:27:29

by kernel test robot

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

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


Attachments:
(No filename) (13.91 kB)
.config.gz (24.41 kB)
Download all attachments

2017-07-26 14:53:29

by Andrew Lunn

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

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 <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2017-07-26 15:10:24

by Andrew Lunn

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

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 <[email protected]>
> ---
> 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

2017-07-28 19:41:01

by John Crispin

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



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 <[email protected]>
>> ---
>> 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

2017-07-28 21:26:05

by Andrew Lunn

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

> 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