[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180708102712.GA662@zaharido-Mobl1.ger.corp.intel.com>
Date: Sun, 8 Jul 2018 12:27:13 +0200
From: "Zahari, Doychev" <zahari.doychev@...el.com>
To: Jianbo Liu <jianbol@...lanox.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
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
[ 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