[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20180709002852.GA16663@mellanox.com>
Date: Mon, 9 Jul 2018 00:28:54 +0000
From: Jianbo Liu <jianbol@...lanox.com>
To: "Zahari, Doychev" <zahari.doychev@...el.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, jiri@...nulli.us,
Jamal Hadi Salim <jhs@...atatu.com>,
Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [PATCH v2 net-next 5/5] net/sched: flower: Add supprt for
matching on QinQ vlan headers
The 07/08/2018 12:27, Zahari, Doychev wrote:
> On Fri, Jul 06, 2018 at 05:38:16AM +0000, Jianbo Liu wrote:
> > As support dissecting of QinQ inner and outer vlan headers, user can
> > add rules to match on QinQ vlan headers.
> >
> > Signed-off-by: Jianbo Liu <jianbol@...lanox.com>
> > Acked-by: Jiri Pirko <jiri@...lanox.com>
> > ---
> > include/uapi/linux/pkt_cls.h | 4 +++
> > net/sched/cls_flower.c | 65 ++++++++++++++++++++++++++++++++++----------
> > 2 files changed, 55 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> > index 84e4c1d..c4262d9 100644
> > --- a/include/uapi/linux/pkt_cls.h
> > +++ b/include/uapi/linux/pkt_cls.h
> > @@ -469,6 +469,10 @@ enum {
> > TCA_FLOWER_KEY_IP_TTL, /* u8 */
> > TCA_FLOWER_KEY_IP_TTL_MASK, /* u8 */
> >
> > + TCA_FLOWER_KEY_CVLAN_ID, /* be16 */
> > + TCA_FLOWER_KEY_CVLAN_PRIO, /* u8 */
> > + TCA_FLOWER_KEY_CVLAN_ETH_TYPE, /* be16 */
> > +
> > __TCA_FLOWER_MAX,
> > };
> >
> > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> > index e93b13d..487a152 100644
> > --- a/net/sched/cls_flower.c
> > +++ b/net/sched/cls_flower.c
> > @@ -35,6 +35,7 @@ struct fl_flow_key {
> > struct flow_dissector_key_basic basic;
> > struct flow_dissector_key_eth_addrs eth;
> > struct flow_dissector_key_vlan vlan;
> > + struct flow_dissector_key_vlan cvlan;
> > union {
> > struct flow_dissector_key_ipv4_addrs ipv4;
> > struct flow_dissector_key_ipv6_addrs ipv6;
> > @@ -449,6 +450,9 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> > [TCA_FLOWER_KEY_IP_TOS_MASK] = { .type = NLA_U8 },
> > [TCA_FLOWER_KEY_IP_TTL] = { .type = NLA_U8 },
> > [TCA_FLOWER_KEY_IP_TTL_MASK] = { .type = NLA_U8 },
> > + [TCA_FLOWER_KEY_CVLAN_ID] = { .type = NLA_U16 },
> > + [TCA_FLOWER_KEY_CVLAN_PRIO] = { .type = NLA_U8 },
> > + [TCA_FLOWER_KEY_CVLAN_ETH_TYPE] = { .type = NLA_U16 },
> > };
> >
> > static void fl_set_key_val(struct nlattr **tb,
> > @@ -501,19 +505,20 @@ static int fl_set_key_mpls(struct nlattr **tb,
> >
> > static void fl_set_key_vlan(struct nlattr **tb,
> > __be16 ethertype,
> > + int vlan_id_key, int vlan_prio_key,
> > struct flow_dissector_key_vlan *key_val,
> > struct flow_dissector_key_vlan *key_mask)
> > {
> > #define VLAN_PRIORITY_MASK 0x7
> >
> > - if (tb[TCA_FLOWER_KEY_VLAN_ID]) {
> > + if (tb[vlan_id_key]) {
> > key_val->vlan_id =
> > - nla_get_u16(tb[TCA_FLOWER_KEY_VLAN_ID]) & VLAN_VID_MASK;
> > + nla_get_u16(tb[vlan_id_key]) & VLAN_VID_MASK;
> > key_mask->vlan_id = VLAN_VID_MASK;
> > }
> > - if (tb[TCA_FLOWER_KEY_VLAN_PRIO]) {
> > + if (tb[vlan_prio_key]) {
> > key_val->vlan_priority =
> > - nla_get_u8(tb[TCA_FLOWER_KEY_VLAN_PRIO]) &
> > + nla_get_u8(tb[vlan_prio_key]) &
> > VLAN_PRIORITY_MASK;
> > key_mask->vlan_priority = VLAN_PRIORITY_MASK;
> > }
> > @@ -596,11 +601,25 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> > ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_ETH_TYPE]);
> >
> > if (eth_type_vlan(ethertype)) {
> > - fl_set_key_vlan(tb, ethertype, &key->vlan, &mask->vlan);
> > - fl_set_key_val(tb, &key->basic.n_proto,
> > - TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> > - &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
> > - sizeof(key->basic.n_proto));
> > + fl_set_key_vlan(tb, ethertype, TCA_FLOWER_KEY_VLAN_ID,
> > + TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan,
> > + &mask->vlan);
> > +
> > + ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]);
>
> Should we not check the nl attribute here? I am getting a crash when issuing the
> command below:
>
> tc filter add dev ens6 ingress prio 1 protocol 802.1q \
> flower vlan_id 100 action vlan push id 200
>
I'll send a fix for it. Thanks.
> [ 5015.082140] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004
> [ 5015.085725] PGD 0 P4D 0
> [ 5015.086338] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 5015.087234] CPU: 0 PID: 277 Comm: tc Not tainted 4.18.0-rc3-ARCH+ #843
> [ 5015.088536] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.11.0-0-g63451fca13-prebuilt.qemu-project.org 04/01/2014
> [ 5015.090900] RIP: 0010:fl_change+0x108a/0x1d00
> [ 5015.091774] Code: 88 97 05 01 00 00 80 8c 24 cd 00 00 00 70 66 41 89 87 06 01 00 00 48 8b 83 c8 00 00 00 b9 ff ff ff
> ff 66 89 8c 24 ce 00 00 00 <0f> b7 40 04 66 3d 81 00 74 0a 66 3d 88 a8 0f 85 23 f3 ff ff 48 8b
> [ 5015.095507] RSP: 0018:ffffc900004cb7b0 EFLAGS: 00010246
> [ 5015.096199] RAX: 0000000000000000 RBX: ffff88003ce06c00 RCX: 00000000ffffffff
> [ 5015.097121] RDX: 0000000000000000 RSI: ffff88003bcdaafe RDI: ffff88003ce06c00
> [ 5015.098034] RBP: ffffc900004cb858 R08: 0000000000000007 R09: 0000000000000006
> [ 5015.099102] R10: ffffffff822e66e0 R11: 0000000000000000 R12: ffff88003bcdaae0
> [ 5015.100173] R13: 0000000000000000 R14: 0000000000000000 R15: ffff88003bcdaa00
> [ 5015.101245] FS: 00007f6d8ff66c00(0000) GS:ffff88003f600000(0000) knlGS:0000000000000000
> [ 5015.102447] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 5015.103297] CR2: 0000000000000004 CR3: 000000003c0a2000 CR4: 00000000000006f0
> [ 5015.104348] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 5015.105333] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 5015.106246] Call Trace:
> [ 5015.106585] tc_new_tfilter+0x3fa/0x640
> [ 5015.107090] rtnetlink_rcv_msg+0x126/0x390
> [ 5015.107626] ? rtnl_calcit.isra.13+0x110/0x110
> [ 5015.108311] netlink_rcv_skb+0x4c/0x120
> [ 5015.108898] netlink_unicast+0x196/0x240
> [ 5015.109496] netlink_sendmsg+0x1f7/0x3c0
> [ 5015.110093] sock_sendmsg+0x19/0x30
> [ 5015.110626] ___sys_sendmsg+0x295/0x2f0
> [ 5015.111211] ? filemap_map_pages+0x19d/0x350
> [ 5015.111863] ? __handle_mm_fault+0xed2/0x1240
> [ 5015.112517] __sys_sendmsg+0x57/0xa0
> [ 5015.113057] do_syscall_64+0x4e/0x100
> [ 5015.113612] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 5015.114361] RIP: 0033:0x7f6d8eecd984
> [ 5015.114917] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 80 00 00 00 00 48 8d 05 41 1e 2c 00 8b 00 85 c0
> 75 13 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 41 89 d4 55 48 89 f5 53
> [ 5015.117748] RSP: 002b:00007ffe110cfce8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [ 5015.118895] RAX: ffffffffffffffda RBX: 000000005b41e4c8 RCX: 00007f6d8eecd984
> [ 5015.119968] RDX: 0000000000000000 RSI: 00007ffe110cfd50 RDI: 0000000000000003
> [ 5015.121041] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
> [ 5015.122107] R10: fffffffffffff78c R11: 0000000000000246 R12: 0000000000000001
> [ 5015.123156] R13: 000055d3bc89c760 R14: 0000000000010081 R15: 00007ffe110d4008
> [ 5015.124205] Modules linked in: joydev mousedev bochs_drm ppdev nls_iso8859_1 ttm nls_cp437 vfat fat drm_kms_helper
> evdev input_leds led_class pcspkr psmouse mac_hid drm parport_pc syscopyarea parport sysfillrect sysimgblt fb_sys_fops
> qemu_fw_cfg intel_agp intel_gtt ia
> [ 5015.129877] CR2: 0000000000000004
> [ 5015.130630] ---[ end trace e68994d35a01ff0f ]---
>
> Is is possible to check for both TPIDs 802.1ad and 802.1q when the vlan_ethtype
> is not set?
>
> Regards
> Zahari
>
> > + if (eth_type_vlan(ethertype)) {
> > + fl_set_key_vlan(tb, ethertype,
> > + TCA_FLOWER_KEY_CVLAN_ID,
> > + TCA_FLOWER_KEY_CVLAN_PRIO,
> > + &key->cvlan, &mask->cvlan);
> > + fl_set_key_val(tb, &key->basic.n_proto,
> > + TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
> > + &mask->basic.n_proto,
> > + TCA_FLOWER_UNSPEC,
> > + sizeof(key->basic.n_proto));
> > + } else {
> > + key->basic.n_proto = ethertype;
> > + mask->basic.n_proto = cpu_to_be16(~0);
> > + }
> > } else {
> > key->basic.n_proto = ethertype;
> > mask->basic.n_proto = cpu_to_be16(~0);
> > @@ -826,6 +845,8 @@ static void fl_init_dissector(struct fl_flow_mask *mask)
> > FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> > FLOW_DISSECTOR_KEY_VLAN, vlan);
> > FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> > + FLOW_DISSECTOR_KEY_CVLAN, cvlan);
> > + FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> > FLOW_DISSECTOR_KEY_ENC_KEYID, enc_key_id);
> > FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
> > FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS, enc_ipv4);
> > @@ -1201,6 +1222,7 @@ static int fl_dump_key_ip(struct sk_buff *skb,
> > }
> >
> > static int fl_dump_key_vlan(struct sk_buff *skb,
> > + int vlan_id_key, int vlan_prio_key,
> > struct flow_dissector_key_vlan *vlan_key,
> > struct flow_dissector_key_vlan *vlan_mask)
> > {
> > @@ -1209,13 +1231,13 @@ static int fl_dump_key_vlan(struct sk_buff *skb,
> > if (!memchr_inv(vlan_mask, 0, sizeof(*vlan_mask)))
> > return 0;
> > if (vlan_mask->vlan_id) {
> > - err = nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ID,
> > + err = nla_put_u16(skb, vlan_id_key,
> > vlan_key->vlan_id);
> > if (err)
> > return err;
> > }
> > if (vlan_mask->vlan_priority) {
> > - err = nla_put_u8(skb, TCA_FLOWER_KEY_VLAN_PRIO,
> > + err = nla_put_u8(skb, vlan_prio_key,
> > vlan_key->vlan_priority);
> > if (err)
> > return err;
> > @@ -1310,13 +1332,28 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
> > if (fl_dump_key_mpls(skb, &key->mpls, &mask->mpls))
> > goto nla_put_failure;
> >
> > - if (fl_dump_key_vlan(skb, &key->vlan, &mask->vlan))
> > + if (fl_dump_key_vlan(skb, TCA_FLOWER_KEY_VLAN_ID,
> > + TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan, &mask->vlan))
> > goto nla_put_failure;
> >
> > - if (mask->vlan.vlan_tpid &&
> > - nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE, key->basic.n_proto))
> > + if (fl_dump_key_vlan(skb, TCA_FLOWER_KEY_CVLAN_ID,
> > + TCA_FLOWER_KEY_CVLAN_PRIO,
> > + &key->cvlan, &mask->cvlan) ||
> > + (mask->cvlan.vlan_tpid &&
> > + nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> > + key->cvlan.vlan_tpid)))
> > goto nla_put_failure;
> >
> > + if (mask->cvlan.vlan_tpid) {
> > + if (nla_put_be16(skb, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
> > + key->basic.n_proto))
> > + goto nla_put_failure;
> > + } else if (mask->vlan.vlan_tpid) {
> > + if (nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> > + key->basic.n_proto))
> > + goto nla_put_failure;
> > + }
> > +
> > if ((key->basic.n_proto == htons(ETH_P_IP) ||
> > key->basic.n_proto == htons(ETH_P_IPV6)) &&
> > (fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
> > --
> > 2.9.5
> >
--
Powered by blists - more mailing lists