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 +, 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
> Acked-by: Jiri Pirko
> ---
> 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, >vlan, >vlan);
> - fl_set_key_val(tb, >basic.n_proto,
> -TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> ->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, >vlan,
> + >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
0004
[ 5015.085725] PGD 0 P4D 0
[ 5015.086338] Oops: [#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:c94cb7b0 EFLAGS: 00010246
[ 5015.096199] RAX: RBX: 88003ce06c00 RCX:
[ 5015.097121] RDX: RSI: 88003bcdaafe RDI: 88003ce06c00
[ 5015.098034] RBP:
[PATCH][next v2] rocker: fix kcalloc parameter order
The function calls to kcalloc use wrong parameter order and incorrect flags
values. GFP_KERNEL is used instead of flags now and the order is corrected.
The change was done using the following coccinelle script:
@@
expression E1,E2;
type T;
@@
-kcalloc(E1, E2, sizeof(T))
+kcalloc(E2, sizeof(T), GFP_KERNEL)
Signed-off-by: Zahari Doychev <zahari.doyc...@linux.com>
---
drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c
b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index da4e26b53a52..0653b70723a3 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -1177,7 +1177,7 @@ static int ofdpa_group_l2_fan_out(struct ofdpa_port
*ofdpa_port,
entry->group_id = group_id;
entry->group_count = group_count;
- entry->group_ids = kcalloc(flags, group_count, sizeof(u32));
+ entry->group_ids = kcalloc(group_count, sizeof(u32), GFP_KERNEL);
if (!entry->group_ids) {
kfree(entry);
return -ENOMEM;
@@ -1456,7 +1456,7 @@ static int ofdpa_port_vlan_flood_group(struct ofdpa_port
*ofdpa_port,
int err = 0;
int i;
- group_ids = kcalloc(flags, port_count, sizeof(u32));
+ group_ids = kcalloc(port_count, sizeof(u32), GFP_KERNEL);
if (!group_ids)
return -ENOMEM;
--
2.13.0
Re: [PATCH][next] rocker: fix kcalloc parameter order
On Sun, Sep 03, 2017 at 10:26:03PM +0200, Zahari Doychev wrote:
> The function calls to kcalloc use wrong parameter order. The flags
> have to be passed as last parameter.
>
> The change was done using the following coccinelle script:
>
> @@
> expression E1,E2;
> type T;
> @@
>
> -kcalloc(E1, E2, sizeof(T))
> +kcalloc(E2, sizeof(T), E1)
>
> Signed-off-by: Zahari Doychev <zahari.doyc...@linux.com>
> ---
> drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c
> b/drivers/net/ethernet/rocker/rocker_ofdpa.c
> index da4e26b53a52..2708451ce22a 100644
> --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
> +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
> @@ -1177,7 +1177,7 @@ static int ofdpa_group_l2_fan_out(struct ofdpa_port
> *ofdpa_port,
> entry->group_id = group_id;
> entry->group_count = group_count;
>
> - entry->group_ids = kcalloc(flags, group_count, sizeof(u32));
> + entry->group_ids = kcalloc(group_count, sizeof(u32), flags);
I think that the flags are also not used correctly, so I will have to resend
the patch.
> if (!entry->group_ids) {
> kfree(entry);
> return -ENOMEM;
> @@ -1456,7 +1456,7 @@ static int ofdpa_port_vlan_flood_group(struct
> ofdpa_port *ofdpa_port,
> int err = 0;
> int i;
>
> - group_ids = kcalloc(flags, port_count, sizeof(u32));
> + group_ids = kcalloc(port_count, sizeof(u32), flags);
> if (!group_ids)
> return -ENOMEM;
>
> --
> 2.13.0
>
[PATCH][next] rocker: fix kcalloc parameter order
The function calls to kcalloc use wrong parameter order. The flags
have to be passed as last parameter.
The change was done using the following coccinelle script:
@@
expression E1,E2;
type T;
@@
-kcalloc(E1, E2, sizeof(T))
+kcalloc(E2, sizeof(T), E1)
Signed-off-by: Zahari Doychev <zahari.doyc...@linux.com>
---
drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c
b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index da4e26b53a52..2708451ce22a 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -1177,7 +1177,7 @@ static int ofdpa_group_l2_fan_out(struct ofdpa_port
*ofdpa_port,
entry->group_id = group_id;
entry->group_count = group_count;
- entry->group_ids = kcalloc(flags, group_count, sizeof(u32));
+ entry->group_ids = kcalloc(group_count, sizeof(u32), flags);
if (!entry->group_ids) {
kfree(entry);
return -ENOMEM;
@@ -1456,7 +1456,7 @@ static int ofdpa_port_vlan_flood_group(struct ofdpa_port
*ofdpa_port,
int err = 0;
int i;
- group_ids = kcalloc(flags, port_count, sizeof(u32));
+ group_ids = kcalloc(port_count, sizeof(u32), flags);
if (!group_ids)
return -ENOMEM;
--
2.13.0
