Message ID | 20170721085813.30789-3-john@phrozen.org |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
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, ð->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
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
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
> 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
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
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(+)