Re: broken behaviour of TC filter delete

[Roman Mashak]

Jiri Pirko  writes:

> Thu, Aug 23, 2018 at 11:39:22PM CEST, m...@mojatatu.com wrote:
>>
>>
>>It appears that the following commit changed the behaviour of scenario where a
>>filter is deleted twice:
>>
>>commit f71e0ca4db187af7c44987e9d21e9042c3046070
>>Author: Jiri Pirko 
>>Date:   Mon Jul 23 09:23:05 2018 +0200
>>
>>net: sched: Avoid implicit chain 0 creation
>>
>>
>>Steps to reproduce :
>>
>>1) create dummy device
>>   $ ip link add dev dummy0 type dummy
>>
>>2) create qdisc
>>   $ tc qdisc add dev dummy0 ingress
>>
>>3) create simple u32 filter with action attached
>>   $ tc filter add dev dummy0 parent : protocol ip prio 1 u32 match ip 
>> src 10.10.10.1/32 action ok
>>
>>4) list the filter
>>   $ tc filter ls dev dummy0 parent :
>>
>>5) delete the filter with the given protocol and priority
>>   $ tc filter del dev dummy0 parent : protocol ip prio 1
>>
>>6) repeat step 5, this will return -ENOENT ("Error: Filter with specified 
>>priority/protocol not found.")
>>However, before the change at step 6 we would get -EINVAL (Error: Cannot find 
>>specified filter chain.)
>>and that makes sense.
>
> Wait, this now returns:
> Error: Cannot find specified filter chain.
> So you want it to return -EINVAL (Error: Cannot find specified filter chain.) 
> ?
> How about for other chains?
>

I must've mixed up the return codes and messages while typing the
message.

So _before_ commit f71e0ca4db187af7c44987e9d21e9042c3046070 step 6 would
return -ENOENT with "Error: Filter with specified priority/protocol not
found." and _after_ the commit it returns -EINVAL (Error: Cannot find
specified filter chain.)

ENOENT seems to be more logical to return when there's no more filter to delete.

broken behaviour of TC filter delete

[Roman Mashak]

It appears that the following commit changed the behaviour of scenario where a
filter is deleted twice:

commit f71e0ca4db187af7c44987e9d21e9042c3046070
Author: Jiri Pirko 
Date:   Mon Jul 23 09:23:05 2018 +0200

net: sched: Avoid implicit chain 0 creation


Steps to reproduce :

1) create dummy device
   $ ip link add dev dummy0 type dummy

2) create qdisc
   $ tc qdisc add dev dummy0 ingress

3) create simple u32 filter with action attached
   $ tc filter add dev dummy0 parent : protocol ip prio 1 u32 match ip src 
10.10.10.1/32 action ok

4) list the filter
   $ tc filter ls dev dummy0 parent :

5) delete the filter with the given protocol and priority
   $ tc filter del dev dummy0 parent : protocol ip prio 1

6) repeat step 5, this will return -ENOENT ("Error: Filter with specified 
priority/protocol not found.")
However, before the change at step 6 we would get -EINVAL (Error: Cannot find 
specified filter chain.)
and that makes sense.

The change breaks a number of our internal TC tests.

Re: how to (cross)connect two (physical) eth ports for ping test?

[Roman Mashak]

"Robert P. J. Day"  writes:

>   (i'm sure this has been explained many times before, so a link
> covering this will almost certainly do just fine.)
>
>   i want to loop one physical ethernet port into another, and just
> ping the daylights from one to the other for stress testing. my fedora
> laptop doesn't actually have two unused ethernet ports, so i just want
> to emulate this by slapping a couple startech USB/net adapters into
> two empty USB ports, setting this up, then doing it all over again
> monday morning on the actual target system, which does have multiple
> ethernet ports.

[...]

I used this in the past to test dual-port NIC over loopback cable, you
will need to ajust the script:

#!/bin/bash -x

ip="sudo $HOME/bin/ip"
eth1=192.168.2.100
eth2=192.168.2.101

dev1=eth1
dev2=eth2
dev1mac=00:1b:21:9b:24:b4
dev2mac=00:1b:21:9b:24:b5

# fake client interfaces and addresses
dev=dummy0
dev_mac=00:00:00:00:00:11

# max fake clients supported for simulation
maxusers=3

## Create dummy device
## Accepted parameters:
##$1 - devname
##$2 - devmac
##$3 - subnet (e.g. 10.10.10)
##$4 - max number of IP addresses to create on interface
setup_dummy()
{
#   sudo sh -c "echo 1 > /proc/sys/net/ipv4/ip_forward"
   # Enable tc hardware offload
#   ethtool -K $SGW_DEV hw-tc-offload on

   $ip link add $1 address $2 type dummy
   $ip link set $1 up
   for i in `seq 1 $4`;
   do
  $ip addr add $3.$i/32 dev $1
   done
}

## Delete dummy device
## Accepted parameters:
##$1 - devname
delete_dummy()
{
  $ip link del $1 type dummy
}

setup_network()
{
  # Send traffic eth3 <-> eth4 over loopback cable, where both interfaces
  # eth3 and eth4 are in the same subnet.
  #
  # We assume that NetworkManager is not running and eth3/eth4 are configured
  # via /etc/network/interfaces:
  #
  # 192.168.1.100/32 dev eth3
  # 192.168.1.101/32 dev eth4
  #
  # Specify source IP address when sending the traffic:
  # ping -I 192.168.1.100 192.168.1.101
  #
  #
  $ip neigh add $eth2 lladdr $dev2mac nud permanent dev $dev1
  $ip neigh add $eth1 lladdr $dev1mac nud permanent dev $dev2
  $ip route add table main $eth1 dev $dev2
  $ip route add table main $eth2 dev $dev1
  $ip rule add from all lookup local pref 100
  $ip rule del pref 0
  $ip rule add from $eth2 to $eth1 iif $dev1 lookup local pref 1
  $ip rule add from $eth1 to $eth2 iif $dev2 lookup local pref 2
  $ip rule add from $eth2 to $eth1 lookup main pref 3
  $ip rule add from $eth1 to $eth2 lookup main pref 4

#  $ip rule add from 10.10.10.0/24 to $eth1 iif $dev1 lookup local pref 5
#  $ip rule add from 10.10.10.0/24 to $eth2 iif $dev2 lookup local pref 6
#  $ip rule add from $eth1 to 10.10.10.0/24 iif $dev2 lookup local pref 7
#  $ip rule add from $eth2 to 10.10.10.0/24 iif $dev1 lookup local pref 8
}

restore_network()
{
  # FIX: hangs connections
  $ip rule flush
  $ip rule add priority 32767 lookup default
}

#delete_dummy dummy0
#delete_dummy dummy1

#setup_dummy dummy0 00:00:00:00:00:11 10.10.10 3
#setup_dummy dummy1 00:00:00:00:00:22 20.20.20 3
setup_network

Re: [PATCH iproute2/next 1/2] tc/act_tunnel_key: Enable setup of tos and ttl

[Roman Mashak]

Or Gerlitz  writes:

> Allow to set tos and ttl for the tunnel.
>
> For example, here's encap rule that sets tos to the tunnel:
>
> tc filter add dev eth0_0 protocol ip parent : prio 10 flower \
>src_mac e4:11:22:33:44:50 dst_mac e4:11:22:33:44:70 \
>action tunnel_key set src_ip 192.168.10.1 dst_ip 192.168.10.2 id 100 
> dst_port 4789 tos 0x30 \
>action mirred egress redirect dev vxlan_sys_4789
>
> Signed-off-by: Or Gerlitz 
> Reviewed-by: Roi Dayan 
> Acked-by: Jiri Pirko 

[...]

Or, could you also update tunnel_key actions for the new options in
$(kernel)/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
once the patches are accepted ?

Re: [PATCH iproute2-next] tc: m_tunnel_key: Add tunnel option support to act_tunnel_key

[Roman Mashak]

Jakub Kicinski  writes:

> From: Simon Horman 
>
> Allow setting tunnel options using the act_tunnel_key action.
>
> Options are expressed as class:type:data and multiple options
> may be listed using a comma delimiter.
>
>  # ip link add name geneve0 type geneve dstport 0 external
>  # tc qdisc add dev eth0 ingress
>  # tc filter add dev eth0 protocol ip parent : \
>  flower indev eth0 \
> ip_proto udp \
> action tunnel_key \
> set src_ip 10.0.99.192 \
> dst_ip 10.0.99.193 \
> dst_port 6081 \
> id 11 \
> geneve_opts 0102:80:00800022,0102:80:00800022 \
> action mirred egress redirect dev geneve0

[...]

Jakub, could you also add relevant tests for the new option in file
$(kernel)/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json?

[PATCH net-next 1/1] net sched actions: add extack messages in pedit action

[Roman Mashak]

Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index ab151346d3d4..55bc96b610e8 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -144,8 +144,10 @@ static int tcf_pedit_init(struct net *net, struct nlattr 
*nla,
int ret = 0, err;
int ksize;
 
-   if (!nla)
+   if (!nla) {
+   NL_SET_ERR_MSG_MOD(extack, "Pedit requires attributes to be 
passed");
return -EINVAL;
+   }
 
err = nla_parse_nested(tb, TCA_PEDIT_MAX, nla, pedit_policy, NULL);
if (err < 0)
@@ -154,21 +156,27 @@ static int tcf_pedit_init(struct net *net, struct nlattr 
*nla,
pattr = tb[TCA_PEDIT_PARMS];
if (!pattr)
pattr = tb[TCA_PEDIT_PARMS_EX];
-   if (!pattr)
+   if (!pattr) {
+   NL_SET_ERR_MSG_MOD(extack, "Missing required TCA_PEDIT_PARMS or 
TCA_PEDIT_PARMS_EX pedit attribute");
return -EINVAL;
+   }
 
parm = nla_data(pattr);
ksize = parm->nkeys * sizeof(struct tc_pedit_key);
-   if (nla_len(pattr) < sizeof(*parm) + ksize)
+   if (nla_len(pattr) < sizeof(*parm) + ksize) {
+   NL_SET_ERR_MSG_ATTR(extack, pattr, "Length of TCA_PEDIT_PARMS 
or TCA_PEDIT_PARMS_EX pedit attribute is invalid");
return -EINVAL;
+   }
 
keys_ex = tcf_pedit_keys_ex_parse(tb[TCA_PEDIT_KEYS_EX], parm->nkeys);
if (IS_ERR(keys_ex))
return PTR_ERR(keys_ex);
 
if (!tcf_idr_check(tn, parm->index, a, bind)) {
-   if (!parm->nkeys)
+   if (!parm->nkeys) {
+   NL_SET_ERR_MSG_MOD(extack, "Pedit requires keys to be 
passed");
return -EINVAL;
+   }
ret = tcf_idr_create(tn, parm->index, est, a,
 _pedit_ops, bind, false);
if (ret)
-- 
2.7.4

[PATCH v2 net-next 3/6] net sched actions: fix sparse warning

[Roman Mashak]

The variable _data in include/asm-generic/sections.h defines sections,
this causes sparse warning in pedit:

net/sched/act_pedit.c:293:35: warning: symbol '_data' shadows an earlier one
./include/asm-generic/sections.h:36:13: originally declared here

Therefore rename the variable.

Reviewed-by: Simon Horman 
Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index e4b29ee79ba8..9c2d8a31a5c5 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -290,7 +290,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
-   u32 *ptr, _data;
+   u32 *ptr, hdata;
int offset = tkey->off;
int hoffset;
u32 val;
@@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
ptr = skb_header_pointer(skb, hoffset + offset,
-4, &_data);
+4, );
if (!ptr)
goto bad;
/* just do it, baby */
@@ -355,7 +355,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
*ptr = ((*ptr & tkey->mask) ^ val);
-   if (ptr == &_data)
+   if (ptr == )
skb_store_bits(skb, hoffset + offset, ptr, 4);
}
 
-- 
2.7.4

[PATCH v2 net-next 2/6] net sched actions: fix coding style in pedit headers

[Roman Mashak]

Fix coding style issues in tc pedit headers detected by the
checkpatch script.

Reviewed-by: Simon Horman 
Signed-off-by: Roman Mashak 
---
 include/net/tc_act/tc_pedit.h| 1 +
 include/uapi/linux/tc_act/tc_pedit.h | 9 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h
index 227a6f1d02f4..fac3ad4a86de 100644
--- a/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h
@@ -17,6 +17,7 @@ struct tcf_pedit {
struct tc_pedit_key *tcfp_keys;
struct tcf_pedit_key_ex *tcfp_keys_ex;
 };
+
 #define to_pedit(a) ((struct tcf_pedit *)a)
 
 static inline bool is_tcf_pedit(const struct tc_action *a)
diff --git a/include/uapi/linux/tc_act/tc_pedit.h 
b/include/uapi/linux/tc_act/tc_pedit.h
index 162d1094c41c..24ec792dacc1 100644
--- a/include/uapi/linux/tc_act/tc_pedit.h
+++ b/include/uapi/linux/tc_act/tc_pedit.h
@@ -17,13 +17,15 @@ enum {
TCA_PEDIT_KEY_EX,
__TCA_PEDIT_MAX
 };
+
 #define TCA_PEDIT_MAX (__TCA_PEDIT_MAX - 1)
-   
 
+
 enum {
TCA_PEDIT_KEY_EX_HTYPE = 1,
TCA_PEDIT_KEY_EX_CMD = 2,
__TCA_PEDIT_KEY_EX_MAX
 };
+
 #define TCA_PEDIT_KEY_EX_MAX (__TCA_PEDIT_KEY_EX_MAX - 1)
 
  /* TCA_PEDIT_KEY_EX_HDR_TYPE_NETWROK is a special case for legacy users. It
@@ -38,6 +40,7 @@ enum pedit_header_type {
TCA_PEDIT_KEY_EX_HDR_TYPE_UDP = 5,
__PEDIT_HDR_TYPE_MAX,
 };
+
 #define TCA_PEDIT_HDR_TYPE_MAX (__PEDIT_HDR_TYPE_MAX - 1)
 
 enum pedit_cmd {
@@ -45,6 +48,7 @@ enum pedit_cmd {
TCA_PEDIT_KEY_EX_CMD_ADD = 1,
__PEDIT_CMD_MAX,
 };
+
 #define TCA_PEDIT_CMD_MAX (__PEDIT_CMD_MAX - 1)
 
 struct tc_pedit_key {
@@ -55,13 +59,14 @@ struct tc_pedit_key {
__u32   offmask;
__u32   shift;
 };
-   
 
+
 struct tc_pedit_sel {
tc_gen;
unsigned char   nkeys;
unsigned char   flags;
struct tc_pedit_key keys[0];
 };
+
 #define tc_pedit tc_pedit_sel
 
 #endif
-- 
2.7.4

[PATCH v2 net-next 5/6] net sched actions: fix misleading text strings in pedit action

[Roman Mashak]

Change "tc filter pedit .." to "tc actions pedit .." in error
messages to clearly refer to pedit action.

Reviewed-by: Simon Horman 
Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 3b775f54cee5..caa6927a992c 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -305,7 +305,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
 
rc = pedit_skb_hdr_offset(skb, htype, );
if (rc) {
-   pr_info("tc filter pedit bad header type 
specified (0x%x)\n",
+   pr_info("tc action pedit bad header type 
specified (0x%x)\n",
htype);
goto bad;
}
@@ -314,7 +314,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
char *d, _d;
 
if (!offset_valid(skb, hoffset + tkey->at)) {
-   pr_info("tc filter pedit 'at' offset %d 
out of bounds\n",
+   pr_info("tc action pedit 'at' offset %d 
out of bounds\n",
hoffset + tkey->at);
goto bad;
}
@@ -326,12 +326,12 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
if (offset % 4) {
-   pr_info("tc filter pedit offset must be on 32 
bit boundaries\n");
+   pr_info("tc action pedit offset must be on 32 
bit boundaries\n");
goto bad;
}
 
if (!offset_valid(skb, hoffset + offset)) {
-   pr_info("tc filter pedit offset %d out of 
bounds\n",
+   pr_info("tc action pedit offset %d out of 
bounds\n",
hoffset + offset);
goto bad;
}
@@ -349,7 +349,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
val = (*ptr + tkey->val) & ~tkey->mask;
break;
default:
-   pr_info("tc filter pedit bad command (%d)\n",
+   pr_info("tc action pedit bad command (%d)\n",
cmd);
goto bad;
}
-- 
2.7.4

[PATCH v2 net-next 0/6] net sched actions: code style cleanup and fixes

[Roman Mashak]

The patchset fixes a few code stylistic issues and typos, as well as one
detected by sparse semantic checker tool.

No functional changes introduced.

Patch 1 & 2 fix coding style bits caught by the checkpatch.pl script
Patch 3 fixes an issue with a shadowed variable
Patch 4 adds sizeof() operator instead of magic number for buffer length
Patch 5 fixes typos in diagnostics messages
Patch 6 explicitly sets unsigned char for bitwise operation

v2:
   - submit for net-next
   - added Reviewed-by tags
   - use u8* instead of char* as per Davide Caratti suggestion

Roman Mashak (6):
  net sched actions: fix coding style in pedit action
  net sched actions: fix coding style in pedit headers
  net sched actions: fix sparse warning
  net sched actions: use sizeof operator for buffer length
  net sched actions: fix misleading text strings in pedit action
  net sched actions: avoid bitwise operation on signed value in pedit

 include/net/tc_act/tc_pedit.h|  1 +
 include/uapi/linux/tc_act/tc_pedit.h |  9 ++--
 net/sched/act_pedit.c| 43 +++-
 3 files changed, 31 insertions(+), 22 deletions(-)

-- 
2.7.4

[PATCH v2 net-next 4/6] net sched actions: use sizeof operator for buffer length

[Roman Mashak]

Replace constant integer with sizeof() to clearly indicate
the destination buffer length in skb_header_pointer() calls.

Reviewed-by: Simon Horman 
Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 9c2d8a31a5c5..3b775f54cee5 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -319,7 +319,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
goto bad;
}
d = skb_header_pointer(skb, hoffset + tkey->at,
-  1, &_d);
+  sizeof(_d), &_d);
if (!d)
goto bad;
offset += (*d & tkey->offmask) >> tkey->shift;
@@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
ptr = skb_header_pointer(skb, hoffset + offset,
-4, );
+sizeof(hdata), );
if (!ptr)
goto bad;
/* just do it, baby */
-- 
2.7.4

[PATCH v2 net-next 1/6] net sched actions: fix coding style in pedit action

[Roman Mashak]

Fix coding style issues in tc pedit action detected by the
checkpatch script.

Reviewed-by: Simon Horman 
Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 8a925c72db5f..e4b29ee79ba8 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -136,15 +136,15 @@ static int tcf_pedit_init(struct net *net, struct nlattr 
*nla,
 {
struct tc_action_net *tn = net_generic(net, pedit_net_id);
struct nlattr *tb[TCA_PEDIT_MAX + 1];
-   struct nlattr *pattr;
-   struct tc_pedit *parm;
-   int ret = 0, err;
-   struct tcf_pedit *p;
struct tc_pedit_key *keys = NULL;
struct tcf_pedit_key_ex *keys_ex;
+   struct tc_pedit *parm;
+   struct nlattr *pattr;
+   struct tcf_pedit *p;
+   int ret = 0, err;
int ksize;
 
-   if (nla == NULL)
+   if (!nla)
return -EINVAL;
 
err = nla_parse_nested(tb, TCA_PEDIT_MAX, nla, pedit_policy, NULL);
@@ -175,7 +175,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr 
*nla,
return ret;
p = to_pedit(*a);
keys = kmalloc(ksize, GFP_KERNEL);
-   if (keys == NULL) {
+   if (!keys) {
tcf_idr_release(*a, bind);
kfree(keys_ex);
return -ENOMEM;
@@ -220,6 +220,7 @@ static void tcf_pedit_cleanup(struct tc_action *a)
 {
struct tcf_pedit *p = to_pedit(a);
struct tc_pedit_key *keys = p->tcfp_keys;
+
kfree(keys);
kfree(p->tcfp_keys_ex);
 }
@@ -284,7 +285,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
if (p->tcfp_nkeys > 0) {
struct tc_pedit_key *tkey = p->tcfp_keys;
struct tcf_pedit_key_ex *tkey_ex = p->tcfp_keys_ex;
-   enum pedit_header_type htype = 
TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
+   enum pedit_header_type htype =
+   TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
@@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
hoffset + tkey->at);
goto bad;
}
-   d = skb_header_pointer(skb, hoffset + tkey->at, 
1,
-  &_d);
+   d = skb_header_pointer(skb, hoffset + tkey->at,
+  1, &_d);
if (!d)
goto bad;
offset += (*d & tkey->offmask) >> tkey->shift;
}
 
if (offset % 4) {
-   pr_info("tc filter pedit"
-   " offset must be on 32 bit 
boundaries\n");
+   pr_info("tc filter pedit offset must be on 32 
bit boundaries\n");
goto bad;
}
 
@@ -335,7 +336,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
goto bad;
}
 
-   ptr = skb_header_pointer(skb, hoffset + offset, 4, 
&_data);
+   ptr = skb_header_pointer(skb, hoffset + offset,
+4, &_data);
if (!ptr)
goto bad;
/* just do it, baby */
@@ -358,8 +360,9 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
goto done;
-   } else
+   } else {
WARN(1, "pedit BUG: index %d\n", p->tcf_index);
+   }
 
 bad:
p->tcf_qstats.overlimits++;
-- 
2.7.4

[PATCH v2 net-next 6/6] net sched actions: avoid bitwise operation on signed value in pedit

[Roman Mashak]

Since char can be unsigned or signed, and bitwise operators may have
implementation-dependent results when performed on signed operands,
declare 'u8 *' operand instead.

Suggested-by: Davide Caratti 
Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index caa6927a992c..ab151346d3d4 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -311,7 +311,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
if (tkey->offmask) {
-   char *d, _d;
+   u8 *d, _d;
 
if (!offset_valid(skb, hoffset + tkey->at)) {
pr_info("tc action pedit 'at' offset %d 
out of bounds\n",
-- 
2.7.4

Re: [PATCH net 1/5] net sched actions: fix coding style in pedit action

[Roman Mashak]

Davide Caratti  writes:

> On Tue, 2018-06-19 at 12:56 -0400, Roman Mashak wrote:
>> Fix coding style issues in tc pedit action detected by the
>> checkpatch script.
>> 
>> Signed-off-by: Roman Mashak 
> ...
>
>> ---
>> @@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
>> tc_action *a,
>>  hoffset + tkey->at);
>>  goto bad;
>>  }
>> -d = skb_header_pointer(skb, hoffset + tkey->at, 
>> 1,
>> -   &_d);
>> +d = skb_header_pointer(skb, hoffset + tkey->at,
>> +   1, &_d);
>>  if (!d)
>>  goto bad;
>>  offset += (*d & tkey->offmask) >> tkey->shift;
>>  }
>
> hello Roman,
>
> nit: while we are here, what about changing the declaration of _d and *d
> to u8, so that the bitwise operation is done on unsigned?

Yes makes sense, I will send v2 in net-next once opened. Thanks Davide.

> BTW: the patch (and the series) looks ok, but I guess it will better
> target net-next when the branch reopens

Re: [PATCH net 5/5] net sched actions: fix misleading text strings in pedit action

[Roman Mashak]

Stephen Hemminger  writes:

> On Tue, 19 Jun 2018 12:56:08 -0400

[...]

>> @@ -326,12 +326,12 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
>> tc_action *a,
>>  }
>>  
>>  if (offset % 4) {
>> -pr_info("tc filter pedit offset must be on 32 
>> bit boundaries\n");
>> +pr_info("tc action pedit offset must be on 32 
>> bit boundaries\n");
>>  goto bad;
>>  }
>>  
>>  if (!offset_valid(skb, hoffset + offset)) {
>> -pr_info("tc filter pedit offset %d out of 
>> bounds\n",
>> +pr_info("tc action pedit offset %d out of 
>> bounds\n",
>>  hoffset + offset);
>>  goto bad;
>
> Time to convert these to netlink extack reporting?

Yes, this is planned in next patches.

[PATCH net 0/5] net sched actions: code style cleanup and fixes

[Roman Mashak]

The patchset fixes a few code stylistic issues and typos, as well as one
detected by sparse semantic checker tool.

No functional changes introduced.

Patch 1 & 2 fix coding style bits caught by the checkpatch.pl script
Patch 3 fixes an issue with a shadowed variable
Patch 4 adds sizeof() operator instead of magic number for buffer length
Patch 5 fixes typos in diagnostics messages

Roman Mashak (5):
  net sched actions: fix coding style in pedit action
  net sched actions: fix coding style in pedit headers
  net sched actions: fix sparse warning
  net sched actions: use sizeof operator for buffer length
  net sched actions: fix misleading text strings in pedit action

 include/net/tc_act/tc_pedit.h|  1 +
 include/uapi/linux/tc_act/tc_pedit.h |  9 ++--
 net/sched/act_pedit.c| 41 +++-
 3 files changed, 30 insertions(+), 21 deletions(-)

-- 
2.7.4

[PATCH net 4/5] net sched actions: use sizeof operator for buffer length

[Roman Mashak]

Replace constant integer with sizeof() to clearly indicate
the destination buffer length in skb_header_pointer() calls.

Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 9c2d8a31a5c5..3b775f54cee5 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -319,7 +319,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
goto bad;
}
d = skb_header_pointer(skb, hoffset + tkey->at,
-  1, &_d);
+  sizeof(_d), &_d);
if (!d)
goto bad;
offset += (*d & tkey->offmask) >> tkey->shift;
@@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
ptr = skb_header_pointer(skb, hoffset + offset,
-4, );
+sizeof(hdata), );
if (!ptr)
goto bad;
/* just do it, baby */
-- 
2.7.4

[PATCH net 3/5] net sched actions: fix sparse warning

[Roman Mashak]

The variable _data in include/asm-generic/sections.h defines sections,
this causes sparse warning in pedit:

net/sched/act_pedit.c:293:35: warning: symbol '_data' shadows an earlier one
./include/asm-generic/sections.h:36:13: originally declared here

Therefore rename the variable.

Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index e4b29ee79ba8..9c2d8a31a5c5 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -290,7 +290,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
-   u32 *ptr, _data;
+   u32 *ptr, hdata;
int offset = tkey->off;
int hoffset;
u32 val;
@@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
ptr = skb_header_pointer(skb, hoffset + offset,
-4, &_data);
+4, );
if (!ptr)
goto bad;
/* just do it, baby */
@@ -355,7 +355,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
*ptr = ((*ptr & tkey->mask) ^ val);
-   if (ptr == &_data)
+   if (ptr == )
skb_store_bits(skb, hoffset + offset, ptr, 4);
}
 
-- 
2.7.4

[PATCH net 2/5] net sched actions: fix coding style in pedit headers

[Roman Mashak]

Fix coding style issues in tc pedit headers detected by the
checkpatch script.

Signed-off-by: Roman Mashak 
---
 include/net/tc_act/tc_pedit.h| 1 +
 include/uapi/linux/tc_act/tc_pedit.h | 9 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h
index 227a6f1d02f4..fac3ad4a86de 100644
--- a/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h
@@ -17,6 +17,7 @@ struct tcf_pedit {
struct tc_pedit_key *tcfp_keys;
struct tcf_pedit_key_ex *tcfp_keys_ex;
 };
+
 #define to_pedit(a) ((struct tcf_pedit *)a)
 
 static inline bool is_tcf_pedit(const struct tc_action *a)
diff --git a/include/uapi/linux/tc_act/tc_pedit.h 
b/include/uapi/linux/tc_act/tc_pedit.h
index 162d1094c41c..24ec792dacc1 100644
--- a/include/uapi/linux/tc_act/tc_pedit.h
+++ b/include/uapi/linux/tc_act/tc_pedit.h
@@ -17,13 +17,15 @@ enum {
TCA_PEDIT_KEY_EX,
__TCA_PEDIT_MAX
 };
+
 #define TCA_PEDIT_MAX (__TCA_PEDIT_MAX - 1)
-   
 
+
 enum {
TCA_PEDIT_KEY_EX_HTYPE = 1,
TCA_PEDIT_KEY_EX_CMD = 2,
__TCA_PEDIT_KEY_EX_MAX
 };
+
 #define TCA_PEDIT_KEY_EX_MAX (__TCA_PEDIT_KEY_EX_MAX - 1)
 
  /* TCA_PEDIT_KEY_EX_HDR_TYPE_NETWROK is a special case for legacy users. It
@@ -38,6 +40,7 @@ enum pedit_header_type {
TCA_PEDIT_KEY_EX_HDR_TYPE_UDP = 5,
__PEDIT_HDR_TYPE_MAX,
 };
+
 #define TCA_PEDIT_HDR_TYPE_MAX (__PEDIT_HDR_TYPE_MAX - 1)
 
 enum pedit_cmd {
@@ -45,6 +48,7 @@ enum pedit_cmd {
TCA_PEDIT_KEY_EX_CMD_ADD = 1,
__PEDIT_CMD_MAX,
 };
+
 #define TCA_PEDIT_CMD_MAX (__PEDIT_CMD_MAX - 1)
 
 struct tc_pedit_key {
@@ -55,13 +59,14 @@ struct tc_pedit_key {
__u32   offmask;
__u32   shift;
 };
-   
 
+
 struct tc_pedit_sel {
tc_gen;
unsigned char   nkeys;
unsigned char   flags;
struct tc_pedit_key keys[0];
 };
+
 #define tc_pedit tc_pedit_sel
 
 #endif
-- 
2.7.4

[PATCH net 5/5] net sched actions: fix misleading text strings in pedit action

[Roman Mashak]

Change "tc filter pedit .." to "tc actions pedit .." in error
messages to clearly refer to pedit action.

Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 3b775f54cee5..caa6927a992c 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -305,7 +305,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
 
rc = pedit_skb_hdr_offset(skb, htype, );
if (rc) {
-   pr_info("tc filter pedit bad header type 
specified (0x%x)\n",
+   pr_info("tc action pedit bad header type 
specified (0x%x)\n",
htype);
goto bad;
}
@@ -314,7 +314,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
char *d, _d;
 
if (!offset_valid(skb, hoffset + tkey->at)) {
-   pr_info("tc filter pedit 'at' offset %d 
out of bounds\n",
+   pr_info("tc action pedit 'at' offset %d 
out of bounds\n",
hoffset + tkey->at);
goto bad;
}
@@ -326,12 +326,12 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
if (offset % 4) {
-   pr_info("tc filter pedit offset must be on 32 
bit boundaries\n");
+   pr_info("tc action pedit offset must be on 32 
bit boundaries\n");
goto bad;
}
 
if (!offset_valid(skb, hoffset + offset)) {
-   pr_info("tc filter pedit offset %d out of 
bounds\n",
+   pr_info("tc action pedit offset %d out of 
bounds\n",
hoffset + offset);
goto bad;
}
@@ -349,7 +349,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
val = (*ptr + tkey->val) & ~tkey->mask;
break;
default:
-   pr_info("tc filter pedit bad command (%d)\n",
+   pr_info("tc action pedit bad command (%d)\n",
cmd);
goto bad;
}
-- 
2.7.4

[PATCH net 1/5] net sched actions: fix coding style in pedit action

[Roman Mashak]

Fix coding style issues in tc pedit action detected by the
checkpatch script.

Signed-off-by: Roman Mashak 
---
 net/sched/act_pedit.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 8a925c72db5f..e4b29ee79ba8 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -136,15 +136,15 @@ static int tcf_pedit_init(struct net *net, struct nlattr 
*nla,
 {
struct tc_action_net *tn = net_generic(net, pedit_net_id);
struct nlattr *tb[TCA_PEDIT_MAX + 1];
-   struct nlattr *pattr;
-   struct tc_pedit *parm;
-   int ret = 0, err;
-   struct tcf_pedit *p;
struct tc_pedit_key *keys = NULL;
struct tcf_pedit_key_ex *keys_ex;
+   struct tc_pedit *parm;
+   struct nlattr *pattr;
+   struct tcf_pedit *p;
+   int ret = 0, err;
int ksize;
 
-   if (nla == NULL)
+   if (!nla)
return -EINVAL;
 
err = nla_parse_nested(tb, TCA_PEDIT_MAX, nla, pedit_policy, NULL);
@@ -175,7 +175,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr 
*nla,
return ret;
p = to_pedit(*a);
keys = kmalloc(ksize, GFP_KERNEL);
-   if (keys == NULL) {
+   if (!keys) {
tcf_idr_release(*a, bind);
kfree(keys_ex);
return -ENOMEM;
@@ -220,6 +220,7 @@ static void tcf_pedit_cleanup(struct tc_action *a)
 {
struct tcf_pedit *p = to_pedit(a);
struct tc_pedit_key *keys = p->tcfp_keys;
+
kfree(keys);
kfree(p->tcfp_keys_ex);
 }
@@ -284,7 +285,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
if (p->tcfp_nkeys > 0) {
struct tc_pedit_key *tkey = p->tcfp_keys;
struct tcf_pedit_key_ex *tkey_ex = p->tcfp_keys_ex;
-   enum pedit_header_type htype = 
TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
+   enum pedit_header_type htype =
+   TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
@@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
hoffset + tkey->at);
goto bad;
}
-   d = skb_header_pointer(skb, hoffset + tkey->at, 
1,
-  &_d);
+   d = skb_header_pointer(skb, hoffset + tkey->at,
+  1, &_d);
if (!d)
goto bad;
offset += (*d & tkey->offmask) >> tkey->shift;
}
 
if (offset % 4) {
-   pr_info("tc filter pedit"
-   " offset must be on 32 bit 
boundaries\n");
+   pr_info("tc filter pedit offset must be on 32 
bit boundaries\n");
goto bad;
}
 
@@ -335,7 +336,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
goto bad;
}
 
-   ptr = skb_header_pointer(skb, hoffset + offset, 4, 
&_data);
+   ptr = skb_header_pointer(skb, hoffset + offset,
+4, &_data);
if (!ptr)
goto bad;
/* just do it, baby */
@@ -358,8 +360,9 @@ static int tcf_pedit(struct sk_buff *skb, const struct 
tc_action *a,
}
 
goto done;
-   } else
+   } else {
WARN(1, "pedit BUG: index %d\n", p->tcf_index);
+   }
 
 bad:
p->tcf_qstats.overlimits++;
-- 
2.7.4

[PATCH iproute2-next 1/1] tc: add missing space symbol in ife output

[Roman Mashak]

In order to make TDC tests match the output patterns, the missing space
character must be added in the mode output string.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_ife.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index 5320e94dbd48..20e9c73d9a0e 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -240,7 +240,7 @@ static int print_ife(struct action_util *au, FILE *f, 
struct rtattr *arg)
p = RTA_DATA(tb[TCA_IFE_PARMS]);
 
print_string(PRINT_ANY, "kind", "%s ", "ife");
-   print_string(PRINT_ANY, "mode", "%s",
+   print_string(PRINT_ANY, "mode", "%s ",
 p->flags & IFE_ENCODE ? "encode" : "decode");
print_action_control(f, "action ", p->action, " ");
 
-- 
2.7.4

[PATCH net-next 1/1] tc-testing: fixed copy-pasting error in ife tests

[Roman Mashak]

Reported-by: Vlad Buslov <vla...@mellanox.com>
Reported-by: Davide Caratti <dcara...@redhat.com>
Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../selftests/tc-testing/tc-tests/actions/ife.json | 28 +++---
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
index 0330ef29..de97e4ff705c 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
@@ -20,7 +20,7 @@
 "matchPattern": "action order [0-9]*: ife encode action pass.*type 
0xED3E.*allow mark.*index 2",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -44,7 +44,7 @@
 "matchPattern": "action order [0-9]*: ife encode action pipe.*type 
0xED3E.*use mark.*index 2",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -68,7 +68,7 @@
 "matchPattern": "action order [0-9]*: ife encode action continue.*type 
0xED3E.*allow mark.*index 2",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -92,7 +92,7 @@
 "matchPattern": "action order [0-9]*: ife encode action drop.*type 
0xED3E.*use mark 789.*index 2",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -116,7 +116,7 @@
 "matchPattern": "action order [0-9]*: ife encode action 
reclassify.*type 0xED3E.*use mark 656768.*index 2",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -140,7 +140,7 @@
 "matchPattern": "action order [0-9]*: ife encode action jump 1.*type 
0xED3E.*use mark 65.*index 2",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -164,7 +164,7 @@
 "matchPattern": "action order [0-9]*: ife encode action 
reclassify.*type 0xED3E.*use mark 4294967295.*index 90",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -210,7 +210,7 @@
 "matchPattern": "action order [0-9]*: ife encode action pass.*type 
0xED3E.*allow prio.*index 9",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -234,7 +234,7 @@
 "matchPattern": "action order [0-9]*: ife encode action pipe.*type 
0xED3E.*use prio 7.*index 9",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -258,7 +258,7 @@
 "matchPattern": "action order [0-9]*: ife encode action continue.*type 
0xED3E.*use prio 3.*index 9",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -282,7 +282,7 @@
 "matchPattern": "action order [0-9]*: ife encode action drop.*type 
0xED3E.*allow prio.*index 9",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -306,7 +306,7 @@
 "matchPattern": "action order [0-9]*: ife encode action 
reclassify.*type 0xED3E.*use prio 998877.*index 9",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -330,7 +330,7 @@
 "matchPattern": "action order [0-9]*: ife encode action jump 10.*type 
0xED3E.*use prio 998877.*index 9",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
@@ -354,7 +354,7 @@
 "matchPattern": "action order [0-9]*: ife encode action 
reclassify.*type 0xED3E.*use prio 4294967295.*index 99",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action skbedit"
+"$TC actions flush action ife"
 ]
 },
 {
-- 
2.7.4

Re: [PATCH 00/14] Modify action API for implementing lockless actions

[Roman Mashak]

Vlad Buslov <vla...@mellanox.com> writes:

> On Wed 16 May 2018 at 14:38, Roman Mashak <m...@mojatatu.com> wrote:
>> On Wed, May 16, 2018 at 2:43 AM, Vlad Buslov <vla...@mellanox.com> wrote:
>>>>>>> I'm trying to run tdc, but keep getting following error even on clean
>>>>>>> branch without my patches:
>>>>>>
>>>>>> Vlad, not sure if you saw my email:
>>>>>> Apply Roman's patch and try again
>>>>>>
>>>>>> https://marc.info/?l=linux-netdev=152639369112020=2
>>>>>>
>>>>>> cheers,
>>>>>> jamal
>>>>>
>>>>> With patch applied I get following error:
>>>>>
>>>>> Test 7d50: Add skbmod action to set destination mac
>>>>> exit: 255 0
>>>>> dst MAC address <11:22:33:44:55:66>
>>>>> RTNETLINK answers: No such file or directory
>>>>> We have an error talking to the kernel
>>>>>
>>>>
>>>> You may actually have broken something with your patches in this case.
>>>
>>> Results is for net-next without my patches.
>>
>> Do you have skbmod compiled in kernel or as a module?
>
> Thanks, already figured out that default config has some actions
> disabled.
> Have more errors now. Everything related to ife:
>
> Test 7682: Create valid ife encode action with mark and pass control
> exit: 255 0
> IFE type 0xED3E
> RTNETLINK answers: No such file or directory
> We have an error talking to the kernel
>
> Test ef47: Create valid ife encode action with mark and pipe control
> exit: 255 0
> IFE type 0xED3E
> RTNETLINK answers: No space left on device
> We have an error talking to the kernel
>
> Test df43: Create valid ife encode action with mark and continue control
> exit: 255 0
> IFE type 0xED3E
> RTNETLINK answers: No space left on device
> We have an error talking to the kernel
>
> Test e4cf: Create valid ife encode action with mark and drop control
> exit: 255 0
> IFE type 0xED3E
> RTNETLINK answers: No space left on device
> We have an error talking to the kernel
>
> Test ccba: Create valid ife encode action with mark and reclassify control
> exit: 255 0
> IFE type 0xED3E
> RTNETLINK answers: No space left on device
> We have an error talking to the kernel
>
> Test a1cf: Create valid ife encode action with mark and jump control
> exit: 255 0
> IFE type 0xED3E
> RTNETLINK answers: No space left on device
> We have an error talking to the kernel
>
> ...
>
>

Please make sure you have these in your kernel config:

CONFIG_NET_ACT_IFE=y
CONFIG_NET_IFE_SKBMARK=m
CONFIG_NET_IFE_SKBPRIO=m
CONFIG_NET_IFE_SKBTCINDEX=m

For tdc to run all the tests, it is assumed that all the supported tc
actions/filters are enabled and compiled.

Re: [PATCH 00/14] Modify action API for implementing lockless actions

[Roman Mashak]

On Wed, May 16, 2018 at 2:43 AM, Vlad Buslov  wrote:
> I'm trying to run tdc, but keep getting following error even on clean
> branch without my patches:

 Vlad, not sure if you saw my email:
 Apply Roman's patch and try again

 https://marc.info/?l=linux-netdev=152639369112020=2

 cheers,
 jamal
>>>
>>> With patch applied I get following error:
>>>
>>> Test 7d50: Add skbmod action to set destination mac
>>> exit: 255 0
>>> dst MAC address <11:22:33:44:55:66>
>>> RTNETLINK answers: No such file or directory
>>> We have an error talking to the kernel
>>>
>>
>> You may actually have broken something with your patches in this case.
>
> Results is for net-next without my patches.

Do you have skbmod compiled in kernel or as a module?

[PATCH net-next 1/1] tc-testing: updated mirred and vlan with more tests

[Roman Mashak]

Added extra test cases for different control actions (reclassify, pipe
etc.), cookies, max values & exceeding maximum, and replace existing
actions unit tests.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/mirred.json|  24 +-
 .../tc-testing/tc-tests/actions/vlan.json  | 320 +++--
 2 files changed, 324 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
index 443c9b3c8664..6e4edfae1799 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
@@ -340,7 +340,7 @@
 },
 {
 "id": "8b69",
-"name": "Add mirred mirror action with maximum index",
+"name": "Add mirred mirror action with index at 32-bit maximum",
 "category": [
 "actions",
 "mirred"
@@ -363,6 +363,28 @@
 ]
 },
 {
+"id": "3f66",
+"name": "Add mirred mirror action with index exceeding 32-bit maximum",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
pipe index 429496729555",
+"expExitCode": "255",
+"verifyCmd": "$TC actions get action mirred index 429496729555",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) pipe.*index 429496729555",
+"matchCount": "0",
+"teardown": []
+},
+{
 "id": "a70e",
 "name": "Delete mirred mirror action",
 "category": [
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
index 4510ddfa6e54..69ea09eefffc 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
@@ -1,7 +1,7 @@
 [
 {
 "id": "6f5a",
-"name": "Add vlan pop action",
+"name": "Add vlan pop action with pipe opcode",
 "category": [
 "actions",
 "vlan"
@@ -14,18 +14,18 @@
 255
 ]
 ],
-"cmdUnderTest": "$TC actions add action vlan pop index 8",
+"cmdUnderTest": "$TC actions add action vlan pop pipe index 8",
 "expExitCode": "0",
 "verifyCmd": "$TC actions list action vlan",
-"matchPattern": "action order [0-9]+: vlan.*pop.*index 8 ref",
+"matchPattern": "action order [0-9]+: vlan.*pop.*pipe.*index 8 ref",
 "matchCount": "1",
 "teardown": [
 "$TC actions flush action vlan"
 ]
 },
 {
-"id": "ee6f",
-"name": "Add vlan pop action with large index",
+"id": "df35",
+"name": "Add vlan pop action with pass opcode",
 "category": [
 "actions",
 "vlan"
@@ -38,10 +38,82 @@
 255
 ]
 ],
-"cmdUnderTest": "$TC actions add action vlan pop index 4294967295",
+"cmdUnderTest": "$TC actions add action vlan pop pass index 8",
 "expExitCode": "0",
-"verifyCmd": "$TC actions list action vlan",
-"matchPattern": "action order [0-9]+: vlan.*pop.*index 4294967295 ref",
+"verifyCmd": "$TC actions get action vlan index 8",
+"matchPattern": "action order [0-9]+: vlan.*pop.*pass.*index 8 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "b0d4",
+"name": "Add vlan pop action with drop opcode",
+"category": [
+"actions",
+"vlan"
+],
+"setu

[PATCH net-next 1/1] tc-testing: fixed copy-pasting error in police tests

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tools/testing/selftests/tc-testing/tc-tests/actions/police.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
index 38d85a1d7492..f03763d81617 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
@@ -401,11 +401,11 @@
 ],
 "cmdUnderTest": "$TC actions add action police rate 10mbit burst 10k 
index 4294967295",
 "expExitCode": "0",
-"verifyCmd": "$TC actions get action mirred index 4294967295",
+"verifyCmd": "$TC actions get action police index 4294967295",
 "matchPattern": "action order [0-9]*:  police 0x rate 10Mbit 
burst 10Kb mtu 2Kb",
 "matchCount": "1",
 "teardown": [
-"$TC actions flush action mirred"
+"$TC actions flush action police"
 ]
 },
 {
-- 
2.7.4

[PATCH net 1/1] net sched actions: fix refcnt leak in skbmod

[Roman Mashak]

When application fails to pass flags in netlink TLV when replacing
existing skbmod action, the kernel will leak refcnt:

$ tc actions get action skbmod index 1
total acts 0

action order 0: skbmod pipe set smac 00:11:22:33:44:55
 index 1 ref 1 bind 0

For example, at this point a buggy application replaces the action with
index 1 with new smac 00:aa:22:33:44:55, it fails because of zero flags,
however refcnt gets bumped:

$ tc actions get actions skbmod index 1
total acts 0

action order 0: skbmod pipe set smac 00:11:22:33:44:55
 index 1 ref 2 bind 0
$

Tha patch fixes this by calling tcf_idr_release() on existing actions.

Fixes: 86da71b57383d ("net_sched: Introduce skbmod action")
Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_skbmod.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index bbcbdce732cc..ad050d7d4b46 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -131,8 +131,11 @@ static int tcf_skbmod_init(struct net *net, struct nlattr 
*nla,
if (exists && bind)
return 0;
 
-   if (!lflags)
+   if (!lflags) {
+   if (exists)
+   tcf_idr_release(*a, bind);
return -EINVAL;
+   }
 
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
-- 
2.7.4

[PATCH v2 net 1/1] net sched actions: fix invalid pointer dereferencing if skbedit flags missing

[Roman Mashak]

When application fails to pass flags in netlink TLV for a new skbedit action,
the kernel results in the following oops:

[8.307732] BUG: unable to handle kernel paging request at 00021130
[8.309167] PGD 8000193d1067 P4D 8000193d1067 PUD 180e0067 PMD 0 
[8.310595] Oops:  [#1] SMP PTI
[8.311334] Modules linked in: kvm_intel kvm irqbypass crct10dif_pclmul 
crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd cryptd 
glue_helper serio_raw
[8.314190] CPU: 1 PID: 397 Comm: tc Not tainted 4.17.0-rc3+ #357
[8.315252] RIP: 0010:__tcf_idr_release+0x33/0x140
[8.316203] RSP: 0018:a0718038f840 EFLAGS: 00010246
[8.317123] RAX: 0001 RBX: 00021100 RCX: 
[8.319831] RDX:  RSI:  RDI: 00021100
[8.321181] RBP:  R08: 0004adf8 R09: 0122
[8.322645] R10:  R11: 9e5b01ed R12: 
[8.324157] R13: 9e0d3cc0 R14:  R15: 
[8.325590] FS:  7f591292e700() GS:8fcf5bc4() 
knlGS:
[8.327001] CS:  0010 DS:  ES:  CR0: 80050033
[8.327987] CR2: 00021130 CR3: 180e6004 CR4: 001606a0
[8.329289] Call Trace:
[8.329735]  tcf_skbedit_init+0xa7/0xb0
[8.330423]  tcf_action_init_1+0x362/0x410
[8.331139]  ? try_to_wake_up+0x44/0x430
[8.331817]  tcf_action_init+0x103/0x190
[8.332511]  tc_ctl_action+0x11a/0x220
[8.333174]  rtnetlink_rcv_msg+0x23d/0x2e0
[8.333902]  ? _cond_resched+0x16/0x40
[8.334569]  ? __kmalloc_node_track_caller+0x5b/0x2c0
[8.335440]  ? rtnl_calcit.isra.31+0xf0/0xf0
[8.336178]  netlink_rcv_skb+0xdb/0x110
[8.336855]  netlink_unicast+0x167/0x220
[8.337550]  netlink_sendmsg+0x2a7/0x390
[8.338258]  sock_sendmsg+0x30/0x40
[8.338865]  ___sys_sendmsg+0x2c5/0x2e0
[8.339531]  ? pagecache_get_page+0x27/0x210
[8.340271]  ? filemap_fault+0xa2/0x630
[8.340943]  ? page_add_file_rmap+0x108/0x200
[8.341732]  ? alloc_set_pte+0x2aa/0x530
[8.342573]  ? finish_fault+0x4e/0x70
[8.343332]  ? __handle_mm_fault+0xbc1/0x10d0
[8.344337]  ? __sys_sendmsg+0x53/0x80
[8.345040]  __sys_sendmsg+0x53/0x80
[8.345678]  do_syscall_64+0x4f/0x100
[8.346339]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[8.347206] RIP: 0033:0x7f591191da67
[8.347831] RSP: 002b:7fff745abd48 EFLAGS: 0246 ORIG_RAX: 
002e
[8.349179] RAX: ffda RBX: 7fff745abe70 RCX: 7f591191da67
[8.350431] RDX:  RSI: 7fff745abdc0 RDI: 0003
[8.351659] RBP: 5af35251 R08: 0001 R09: 
[8.352922] R10: 05f1 R11: 0246 R12: 
[8.354183] R13: 7fff745afed0 R14: 0001 R15: 006767c0
[8.355400] Code: 41 89 d4 53 89 f5 48 89 fb e8 aa 20 fd ff 85 c0 0f 84 ed 00
00 00 48 85 db 0f 84 cf 00 00 00 40 84 ed 0f 85 cd 00 00 00 45 84 e4 <8b> 53 30
74 0d 85 d2 b8 ff ff ff ff 0f 8f b3 00 00 00 8b 43 2c 
[8.358699] RIP: __tcf_idr_release+0x33/0x140 RSP: a0718038f840
[8.359770] CR2: 00021130
[8.360438] ---[ end trace 60c66be45dfc14f0 ]---

The caller calls action's ->init() and passes pointer to "struct tc_action *a",
which later may be initialized to point at the existing action, otherwise
"struct tc_action *a" is still invalid, and therefore dereferencing it is an
error as happens in tcf_idr_release, where refcnt is decremented.

So in case of missing flags tcf_idr_release must be called only for
existing actions.

v2:
- prepare patch for net tree

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_skbedit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index ddf69fc01bdf..6138d1d71900 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -121,7 +121,8 @@ static int tcf_skbedit_init(struct net *net, struct nlattr 
*nla,
return 0;
 
if (!flags) {
-   tcf_idr_release(*a, bind);
+   if (exists)
+   tcf_idr_release(*a, bind);
return -EINVAL;
}
 
-- 
2.7.4

[PATCH net-next 1/1] net sched actions: fix invalid pointer dereferencing if skbedit flags missing

[Roman Mashak]

When application fails to pass flags in netlink TLV for a new skbedit action,
the kernel results in the following oops:

[8.307732] BUG: unable to handle kernel paging request at 00021130
[8.309167] PGD 8000193d1067 P4D 8000193d1067 PUD 180e0067 PMD 0 
[8.310595] Oops:  [#1] SMP PTI
[8.311334] Modules linked in: kvm_intel kvm irqbypass crct10dif_pclmul 
crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd cryptd 
glue_helper serio_raw
[8.314190] CPU: 1 PID: 397 Comm: tc Not tainted 4.17.0-rc3+ #357
[8.315252] RIP: 0010:__tcf_idr_release+0x33/0x140
[8.316203] RSP: 0018:a0718038f840 EFLAGS: 00010246
[8.317123] RAX: 0001 RBX: 00021100 RCX: 
[8.319831] RDX:  RSI:  RDI: 00021100
[8.321181] RBP:  R08: 0004adf8 R09: 0122
[8.322645] R10:  R11: 9e5b01ed R12: 
[8.324157] R13: 9e0d3cc0 R14:  R15: 
[8.325590] FS:  7f591292e700() GS:8fcf5bc4() 
knlGS:
[8.327001] CS:  0010 DS:  ES:  CR0: 80050033
[8.327987] CR2: 00021130 CR3: 180e6004 CR4: 001606a0
[8.329289] Call Trace:
[8.329735]  tcf_skbedit_init+0xa7/0xb0
[8.330423]  tcf_action_init_1+0x362/0x410
[8.331139]  ? try_to_wake_up+0x44/0x430
[8.331817]  tcf_action_init+0x103/0x190
[8.332511]  tc_ctl_action+0x11a/0x220
[8.333174]  rtnetlink_rcv_msg+0x23d/0x2e0
[8.333902]  ? _cond_resched+0x16/0x40
[8.334569]  ? __kmalloc_node_track_caller+0x5b/0x2c0
[8.335440]  ? rtnl_calcit.isra.31+0xf0/0xf0
[8.336178]  netlink_rcv_skb+0xdb/0x110
[8.336855]  netlink_unicast+0x167/0x220
[8.337550]  netlink_sendmsg+0x2a7/0x390
[8.338258]  sock_sendmsg+0x30/0x40
[8.338865]  ___sys_sendmsg+0x2c5/0x2e0
[8.339531]  ? pagecache_get_page+0x27/0x210
[8.340271]  ? filemap_fault+0xa2/0x630
[8.340943]  ? page_add_file_rmap+0x108/0x200
[8.341732]  ? alloc_set_pte+0x2aa/0x530
[8.342573]  ? finish_fault+0x4e/0x70
[8.343332]  ? __handle_mm_fault+0xbc1/0x10d0
[8.344337]  ? __sys_sendmsg+0x53/0x80
[8.345040]  __sys_sendmsg+0x53/0x80
[8.345678]  do_syscall_64+0x4f/0x100
[8.346339]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[8.347206] RIP: 0033:0x7f591191da67
[8.347831] RSP: 002b:7fff745abd48 EFLAGS: 0246 ORIG_RAX: 
002e
[8.349179] RAX: ffda RBX: 7fff745abe70 RCX: 7f591191da67
[8.350431] RDX:  RSI: 7fff745abdc0 RDI: 0003
[8.351659] RBP: 5af35251 R08: 0001 R09: 
[8.352922] R10: 05f1 R11: 0246 R12: 
[8.354183] R13: 7fff745afed0 R14: 0001 R15: 006767c0
[8.355400] Code: 41 89 d4 53 89 f5 48 89 fb e8 aa 20 fd ff 85 c0 0f 84 ed 00
00 00 48 85 db 0f 84 cf 00 00 00 40 84 ed 0f 85 cd 00 00 00 45 84 e4 <8b> 53 30
74 0d 85 d2 b8 ff ff ff ff 0f 8f b3 00 00 00 8b 43 2c 
[8.358699] RIP: __tcf_idr_release+0x33/0x140 RSP: a0718038f840
[8.359770] CR2: 00021130
[8.360438] ---[ end trace 60c66be45dfc14f0 ]---

The caller calls action's ->init() and passes pointer to "struct tc_action *a",
which later may initialized to point at the existing action, otherwise
"struct tc_action *a" is still invalid, and therefore dereferencing it is error.

So checking flags should be done as early as possible, before idr lookups.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_skbedit.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index ddf69fc01bdf..6c88037faf51 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -114,17 +114,15 @@ static int tcf_skbedit_init(struct net *net, struct 
nlattr *nla,
mask = nla_data(tb[TCA_SKBEDIT_MASK]);
}
 
+   if (!flags)
+   return -EINVAL;
+
parm = nla_data(tb[TCA_SKBEDIT_PARMS]);
 
exists = tcf_idr_check(tn, parm->index, a, bind);
if (exists && bind)
return 0;
 
-   if (!flags) {
-   tcf_idr_release(*a, bind);
-   return -EINVAL;
-   }
-
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
 _skbedit_ops, bind, false);
-- 
2.7.4

[PATCH net-next 1/1] tc-testing: updated ife test cases

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../selftests/tc-testing/tc-tests/actions/ife.json | 1036 +++-
 1 file changed, 1024 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
index 9f34f0753969..0330ef29 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
@@ -1,7 +1,7 @@
 [
 {
-"id": "a568",
-"name": "Add action with ife type",
+"id": "7682",
+"name": "Create valid ife encode action with mark and pass control",
 "category": [
 "actions",
 "ife"
@@ -12,21 +12,878 @@
 0,
 1,
 255
-],
-"$TC actions add action ife encode type 0xDEAD index 1"
+]
 ],
-"cmdUnderTest": "$TC actions get action ife index 1",
+"cmdUnderTest": "$TC actions add action ife encode allow mark pass 
index 2",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action ife index 2",
+"matchPattern": "action order [0-9]*: ife encode action pass.*type 
0xED3E.*allow mark.*index 2",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action skbedit"
+]
+},
+{
+"id": "ef47",
+"name": "Create valid ife encode action with mark and pipe control",
+"category": [
+"actions",
+"ife"
+],
+"setup": [
+[
+"$TC actions flush action ife",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action ife encode use mark 10 pipe 
index 2",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action ife index 2",
+"matchPattern": "action order [0-9]*: ife encode action pipe.*type 
0xED3E.*use mark.*index 2",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action skbedit"
+]
+},
+{
+"id": "df43",
+"name": "Create valid ife encode action with mark and continue 
control",
+"category": [
+"actions",
+"ife"
+],
+"setup": [
+[
+"$TC actions flush action ife",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action ife encode allow mark continue 
index 2",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action ife index 2",
+"matchPattern": "action order [0-9]*: ife encode action continue.*type 
0xED3E.*allow mark.*index 2",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action skbedit"
+]
+},
+{
+"id": "e4cf",
+"name": "Create valid ife encode action with mark and drop control",
+"category": [
+"actions",
+"ife"
+],
+"setup": [
+[
+"$TC actions flush action ife",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action ife encode use mark 789 drop 
index 2",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action ife index 2",
+"matchPattern": "action order [0-9]*: ife encode action drop.*type 
0xED3E.*use mark 789.*index 2",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action skbedit"
+]
+},
+{
+"id": "ccba",
+"name": "Create valid ife encode action with mark and reclassify 
control",
+"category": [
+"actions",
+"ife"
+],
+"setup": [
+[
+"$TC actions flush action ife",
+0,
+

[PATCH iproute2 1/1] tc: return on invalid smac or dmac in ife action

[Roman Mashak]

Return on invalid smac/dmac and use invarg consistently for invalid
arguments report.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_ife.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index d7e61703f666..ed0913a379aa 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -94,9 +94,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char 
***argv_p,
} else if (matches(*argv, "tcindex") == 0) {
ife_tcindex = IFE_META_TCINDEX;
} else {
-   fprintf(stderr, "Illegal meta define <%s>\n",
-   *argv);
-   return -1;
+   invarg("Illegal meta define", *argv);
}
} else if (matches(*argv, "use") == 0) {
NEXT_ARG();
@@ -116,9 +114,7 @@ static int parse_ife(struct action_util *a, int *argc_p, 
char ***argv_p,
invarg("ife tcindex val is invalid",
   *argv);
} else {
-   fprintf(stderr, "Illegal meta use type <%s>\n",
-   *argv);
-   return -1;
+   invarg("Illegal meta use type", *argv);
}
} else if (matches(*argv, "type") == 0) {
NEXT_ARG();
@@ -132,8 +128,7 @@ static int parse_ife(struct action_util *a, int *argc_p, 
char ***argv_p,
if (sscanf(daddr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
   dbuf, dbuf + 1, dbuf + 2,
   dbuf + 3, dbuf + 4, dbuf + 5) != 6) {
-   fprintf(stderr, "Invalid mac address %s\n",
-   daddr);
+   invarg("Invalid mac address", *argv);
}
fprintf(stderr, "dst MAC address <%s>\n", daddr);
 
@@ -143,8 +138,7 @@ static int parse_ife(struct action_util *a, int *argc_p, 
char ***argv_p,
if (sscanf(saddr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
   sbuf, sbuf + 1, sbuf + 2,
   sbuf + 3, sbuf + 4, sbuf + 5) != 6) {
-   fprintf(stderr, "Invalid mac address %s\n",
-   saddr);
+   invarg("Invalid mac address", *argv);
}
fprintf(stderr, "src MAC address <%s>\n", saddr);
} else if (matches(*argv, "help") == 0) {
-- 
2.7.4

[PATCH net-next 1/1] tc-testing: add sample action tests

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/sample.json| 588 +
 1 file changed, 588 insertions(+)
 create mode 100644 
tools/testing/selftests/tc-testing/tc-tests/actions/sample.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json
new file mode 100644
index ..3aca33c00039
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json
@@ -0,0 +1,588 @@
+[
+{
+"id": "9784",
+"name": "Add valid sample action with mandatory arguments",
+"category": [
+"actions",
+"sample"
+],
+"setup": [
+[
+"$TC actions flush action sample",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action sample rate 10 group 1 index 
2",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action sample index 2",
+"matchPattern": "action order [0-9]+: sample rate 1/10 group 1.*index 
2 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action sample"
+]
+},
+{
+"id": "5c91",
+"name": "Add valid sample action with mandatory arguments and continue 
control action",
+"category": [
+"actions",
+"sample"
+],
+"setup": [
+[
+"$TC actions flush action sample",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action sample rate 700 group 2 
continue index 2",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action sample index 2",
+"matchPattern": "action order [0-9]+: sample rate 1/700 group 2 
continue.*index 2 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action sample"
+]
+},
+{
+"id": "334b",
+"name": "Add valid sample action with mandatory arguments and drop 
control action",
+"category": [
+"actions",
+"sample"
+],
+"setup": [
+[
+"$TC actions flush action sample",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action sample rate 1 group 11 
drop index 22",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action sample",
+"matchPattern": "action order [0-9]+: sample rate 1/1 group 11 
drop.*index 22 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action sample"
+]
+},
+{
+"id": "da69",
+"name": "Add valid sample action with mandatory arguments and 
reclassify control action",
+"category": [
+"actions",
+"sample"
+],
+"setup": [
+[
+"$TC actions flush action sample",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action sample rate 2 group 72 
reclassify index 100",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action sample",
+"matchPattern": "action order [0-9]+: sample rate 1/2 group 72 
reclassify.*index 100 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action sample"
+]
+},
+{
+"id": "13ce",
+"name": "Add valid sample action with mandatory arguments and pipe 
control action",
+"category": [
+"actions",
+"sample"
+],
+"setup": [
+[
+"$TC actions flush action sample",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "

[PATCH iproute2-next 1/1] tc: jsonify ife action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_ife.c | 54 --
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index d7e61703f666..15d09a167450 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -240,22 +240,24 @@ static int print_ife(struct action_util *au, FILE *f, 
struct rtattr *arg)
parse_rtattr_nested(tb, TCA_IFE_MAX, arg);
 
if (tb[TCA_IFE_PARMS] == NULL) {
-   fprintf(f, "[NULL ife parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL ife parameters]");
return -1;
}
p = RTA_DATA(tb[TCA_IFE_PARMS]);
 
-   fprintf(f, "ife %s ", p->flags & IFE_ENCODE ? "encode" : "decode");
+   print_string(PRINT_ANY, "kind", "%s ", "ife");
+   print_string(PRINT_ANY, "mode", "%s",
+p->flags & IFE_ENCODE ? "encode" : "decode");
print_action_control(f, "action ", p->action, " ");
 
if (tb[TCA_IFE_TYPE]) {
ife_type = rta_getattr_u16(tb[TCA_IFE_TYPE]);
has_optional = 1;
-   fprintf(f, "type 0x%X ", ife_type);
+   print_0xhex(PRINT_ANY, "type", "type 0x%X ", ife_type);
}
 
if (has_optional)
-   fprintf(f, "\n\t ");
+   print_string(PRINT_FP, NULL, "%s\t", _SL_);
 
if (tb[TCA_IFE_METALST]) {
struct rtattr *metalist[IFE_META_MAX + 1];
@@ -268,9 +270,11 @@ static int print_ife(struct action_util *au, FILE *f, 
struct rtattr *arg)
len = RTA_PAYLOAD(metalist[IFE_META_SKBMARK]);
if (len) {
mmark = 
rta_getattr_u32(metalist[IFE_META_SKBMARK]);
-   fprintf(f, "use mark %u ", mmark);
+   print_uint(PRINT_ANY, "mark", "use mark %u ",
+  mmark);
} else
-   fprintf(f, "allow mark ");
+   print_string(PRINT_ANY, "mark", "%s mark ",
+"allow");
}
 
if (metalist[IFE_META_TCINDEX]) {
@@ -278,41 +282,47 @@ static int print_ife(struct action_util *au, FILE *f, 
struct rtattr *arg)
if (len) {
mtcindex =

rta_getattr_u16(metalist[IFE_META_TCINDEX]);
-   fprintf(f, "use tcindex %d ", mtcindex);
+   print_uint(PRINT_ANY, "tcindex",
+  "use tcindex %u ", mtcindex);
} else
-   fprintf(f, "allow tcindex ");
+   print_string(PRINT_ANY, "tcindex",
+"%s tcindex ", "allow");
}
 
if (metalist[IFE_META_PRIO]) {
len = RTA_PAYLOAD(metalist[IFE_META_PRIO]);
if (len) {
mprio = 
rta_getattr_u32(metalist[IFE_META_PRIO]);
-   fprintf(f, "use prio %u ", mprio);
+   print_uint(PRINT_ANY, "prio", "use prio %u ",
+  mprio);
} else
-   fprintf(f, "allow prio ");
+   print_string(PRINT_ANY, "prio", "%s prio ",
+"allow");
}
 
}
 
if (tb[TCA_IFE_DMAC]) {
has_optional = 1;
-   fprintf(f, "dst %s ",
-   ll_addr_n2a(RTA_DATA(tb[TCA_IFE_DMAC]),
-   RTA_PAYLOAD(tb[TCA_IFE_DMAC]), 0, b2,
-   sizeof(b2)));
-
+   print_string(PRINT_ANY, "dst", "dst %s ",
+ll_addr_n2a(RTA_DATA(tb[TCA_IFE_DMAC]),
+RTA_PAYLOAD(tb[TCA_IFE_DMAC]), 0, b2,
+sizeof(b2)));
}
 
if (tb[TCA_IFE_SMAC]) {
has_optional = 1;
-   fprintf(f, "src %s ",
-   ll_addr_n2a(RTA_DATA(tb[TCA_IFE_SMAC]),
-   RTA_PAYLOAD(tb[TCA_IFE_SMAC]), 0, b2,
-   sizeof(b2)));
+ 

[PATCH v2 iproute2-next 1/1] tc: jsonify skbedit action

[Roman Mashak]

v2:
   FIxed strings format in print_string()

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_skbedit.c | 53 +
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c
index db5c64caf2ba..7391fc7f158c 100644
--- a/tc/m_skbedit.c
+++ b/tc/m_skbedit.c
@@ -168,9 +168,8 @@ static int print_skbedit(struct action_util *au, FILE *f, 
struct rtattr *arg)
struct rtattr *tb[TCA_SKBEDIT_MAX + 1];
 
SPRINT_BUF(b1);
-   __u32 *priority;
-   __u32 *mark;
-   __u16 *queue_mapping, *ptype;
+   __u32 priority;
+   __u16 ptype;
struct tc_skbedit *p = NULL;
 
if (arg == NULL)
@@ -179,43 +178,49 @@ static int print_skbedit(struct action_util *au, FILE *f, 
struct rtattr *arg)
parse_rtattr_nested(tb, TCA_SKBEDIT_MAX, arg);
 
if (tb[TCA_SKBEDIT_PARMS] == NULL) {
-   fprintf(f, "[NULL skbedit parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL skbedit parameters]");
return -1;
}
p = RTA_DATA(tb[TCA_SKBEDIT_PARMS]);
 
-   fprintf(f, " skbedit");
+   print_string(PRINT_ANY, "kind", "%s ", "skbedit");
 
if (tb[TCA_SKBEDIT_QUEUE_MAPPING] != NULL) {
-   queue_mapping = RTA_DATA(tb[TCA_SKBEDIT_QUEUE_MAPPING]);
-   fprintf(f, " queue_mapping %u", *queue_mapping);
+   print_uint(PRINT_ANY, "queue_mapping", "queue_mapping %u",
+  rta_getattr_u16(tb[TCA_SKBEDIT_QUEUE_MAPPING]));
}
if (tb[TCA_SKBEDIT_PRIORITY] != NULL) {
-   priority = RTA_DATA(tb[TCA_SKBEDIT_PRIORITY]);
-   fprintf(f, " priority %s", sprint_tc_classid(*priority, b1));
+   priority = rta_getattr_u32(tb[TCA_SKBEDIT_PRIORITY]);
+   print_string(PRINT_ANY, "priority", " priority %s",
+sprint_tc_classid(priority, b1));
}
if (tb[TCA_SKBEDIT_MARK] != NULL) {
-   mark = RTA_DATA(tb[TCA_SKBEDIT_MARK]);
-   fprintf(f, " mark %d", *mark);
+   print_uint(PRINT_ANY, "mark", " mark %u",
+  rta_getattr_u32(tb[TCA_SKBEDIT_MARK]));
}
if (tb[TCA_SKBEDIT_PTYPE] != NULL) {
-   ptype = RTA_DATA(tb[TCA_SKBEDIT_PTYPE]);
-   if (*ptype == PACKET_HOST)
-   fprintf(f, " ptype host");
-   else if (*ptype == PACKET_BROADCAST)
-   fprintf(f, " ptype broadcast");
-   else if (*ptype == PACKET_MULTICAST)
-   fprintf(f, " ptype multicast");
-   else if (*ptype == PACKET_OTHERHOST)
-   fprintf(f, " ptype otherhost");
+   ptype = rta_getattr_u16(tb[TCA_SKBEDIT_PTYPE]);
+   if (ptype == PACKET_HOST)
+   print_string(PRINT_ANY, "ptype", " ptype %s", "host");
+   else if (ptype == PACKET_BROADCAST)
+   print_string(PRINT_ANY, "ptype", " ptype %s",
+"broadcast");
+   else if (ptype == PACKET_MULTICAST)
+   print_string(PRINT_ANY, "ptype", " ptype %s",
+"multicast");
+   else if (ptype == PACKET_OTHERHOST)
+   print_string(PRINT_ANY, "ptype", " ptype %s",
+"otherhost");
else
-   fprintf(f, " ptype %d", *ptype);
+   print_uint(PRINT_ANY, "ptype", " ptype %u", ptype);
}
 
print_action_control(f, " ", p->action, "");
 
-   fprintf(f, "\n\t index %u ref %d bind %d",
-   p->index, p->refcnt, p->bindcnt);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_uint(PRINT_ANY, "index", "\t index %u", p->index);
+   print_int(PRINT_ANY, "ref", " ref %d", p->refcnt);
+   print_int(PRINT_ANY, "bind", " bind %d", p->bindcnt);
 
if (show_stats) {
if (tb[TCA_SKBEDIT_TM]) {
@@ -225,7 +230,7 @@ static int print_skbedit(struct action_util *au, FILE *f, 
struct rtattr *arg)
}
}
 
-   fprintf(f, "\n ");
+   print_string(PRINT_FP, NULL, "%s", _SL_);
 
return 0;
 }
-- 
2.7.4

Re: [PATCH iproute2-next 1/1] tc: jsonify skbedit action

[Roman Mashak]

David Ahern <dsah...@gmail.com> writes:

> On 4/3/18 1:24 PM, Roman Mashak wrote:
>>  if (tb[TCA_SKBEDIT_PTYPE] != NULL) {
>> -ptype = RTA_DATA(tb[TCA_SKBEDIT_PTYPE]);
>> -if (*ptype == PACKET_HOST)
>> -fprintf(f, " ptype host");
>> -else if (*ptype == PACKET_BROADCAST)
>> -fprintf(f, " ptype broadcast");
>> -else if (*ptype == PACKET_MULTICAST)
>> -fprintf(f, " ptype multicast");
>> -else if (*ptype == PACKET_OTHERHOST)
>> -fprintf(f, " ptype otherhost");
>> +ptype = rta_getattr_u16(tb[TCA_SKBEDIT_PTYPE]);
>> +if (ptype == PACKET_HOST)
>> +print_string(PRINT_ANY, "ptype", " %s", "ptype host");
>> +else if (ptype == PACKET_BROADCAST)
>> +print_string(PRINT_ANY, "ptype", " %s",
>> + "ptype broadcast");
>> +else if (ptype == PACKET_MULTICAST)
>> +print_string(PRINT_ANY, "ptype", " %s",
>> + "ptype multicast");
>> +else if (ptype == PACKET_OTHERHOST)
>> +print_string(PRINT_ANY, "ptype", " %s",
>> + "ptype otherhost");
>
> Shouldn't that be:
> print_string(PRINT_ANY, "ptype", "ptype %s", "otherhost");
>
> And ditto for the other strings.
>
>>  else
>> -fprintf(f, " ptype %d", *ptype);
>> +print_uint(PRINT_ANY, "ptype", " %u", ptype);
>
> And then this one needs 'ptype' before %u

OK. I will send v2.

Re: [PATCH iproute2-next] tc: Correct json output for actions

[Roman Mashak]

Yuval Mintz <yuv...@mellanox.com> writes:

> Commit 9fd3f0b255d9 ("tc: enable json output for actions") added JSON
> support for tc-actions at the expense of breaking other use cases that
> reach tc_print_action(), as the latter don't expect the 'actions' array
> to be a new object.
>
> Consider the following taken duringrun of tc_chain.sh selftest,
> and see the latter command output is broken:
>
> $ ./tc/tc -j -p actions list action gact | grep -C 3 actions
> [ {
> "total acts": 1
> },{
> "actions": [ {
> "order": 0,
>
> $ ./tc/tc -p -j -s filter show dev enp3s0np2 ingress | grep -C 3 actions
> },
> "skip_hw": true,
> "not_in_hw": true,{
> "actions": [ {
> "order": 1,
> "kind": "gact",
> "control_action": {
>
> Relocate the open/close of the JSON object to declare the object only
> for the case that needs it.
>
> Signed-off-by: Yuval Mintz <yuv...@mellanox.com>

[...]


Good catch, thanks Yuval.

Tested-by: Roman Mashak <m...@mojatatu.com>

[PATCH iproute2-next 1/1] tc: jsonify tunnel_key action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_tunnel_key.c | 36 +---
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index bac3c07fa90b..0fa461549ad9 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -221,7 +221,13 @@ static void tunnel_key_print_ip_addr(FILE *f, const char 
*name,
else
return;
 
-   fprintf(f, "\n\t%s %s", name, rt_addr_n2a_rta(family, attr));
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   if (matches(name, "src_ip") == 0)
+   print_string(PRINT_ANY, "src_ip", "\tsrc_ip %s",
+rt_addr_n2a_rta(family, attr));
+   else if (matches(name, "dst_ip") == 0)
+   print_string(PRINT_ANY, "dst_ip", "\tdst_ip %s",
+rt_addr_n2a_rta(family, attr));
 }
 
 static void tunnel_key_print_key_id(FILE *f, const char *name,
@@ -229,7 +235,8 @@ static void tunnel_key_print_key_id(FILE *f, const char 
*name,
 {
if (!attr)
return;
-   fprintf(f, "\n\t%s %d", name, rta_getattr_be32(attr));
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_uint(PRINT_ANY, "key_id", "\tkey_id %u", rta_getattr_be32(attr));
 }
 
 static void tunnel_key_print_dst_port(FILE *f, char *name,
@@ -237,7 +244,9 @@ static void tunnel_key_print_dst_port(FILE *f, char *name,
 {
if (!attr)
return;
-   fprintf(f, "\n\t%s %d", name, rta_getattr_be16(attr));
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_uint(PRINT_ANY, "dst_port", "\tdst_port %u",
+  rta_getattr_be16(attr));
 }
 
 static void tunnel_key_print_flag(FILE *f, const char *name_on,
@@ -246,7 +255,9 @@ static void tunnel_key_print_flag(FILE *f, const char 
*name_on,
 {
if (!attr)
return;
-   fprintf(f, "\n\t%s", rta_getattr_u8(attr) ? name_on : name_off);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_string(PRINT_ANY, "flag", "\t%s",
+rta_getattr_u8(attr) ? name_on : name_off);
 }
 
 static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr 
*arg)
@@ -260,19 +271,20 @@ static int print_tunnel_key(struct action_util *au, FILE 
*f, struct rtattr *arg)
parse_rtattr_nested(tb, TCA_TUNNEL_KEY_MAX, arg);
 
if (!tb[TCA_TUNNEL_KEY_PARMS]) {
-   fprintf(f, "[NULL tunnel_key parameters]");
+   print_string(PRINT_FP, NULL, "%s",
+"[NULL tunnel_key parameters]");
return -1;
}
parm = RTA_DATA(tb[TCA_TUNNEL_KEY_PARMS]);
 
-   fprintf(f, "tunnel_key");
+   print_string(PRINT_ANY, "kind", "%s ", "tunnel_key");
 
switch (parm->t_action) {
case TCA_TUNNEL_KEY_ACT_RELEASE:
-   fprintf(f, " unset");
+   print_string(PRINT_ANY, "mode", " %s", "unset");
break;
case TCA_TUNNEL_KEY_ACT_SET:
-   fprintf(f, " set");
+   print_string(PRINT_ANY, "mode", " %s", "set");
tunnel_key_print_ip_addr(f, "src_ip",
 tb[TCA_TUNNEL_KEY_ENC_IPV4_SRC]);
tunnel_key_print_ip_addr(f, "dst_ip",
@@ -291,8 +303,10 @@ static int print_tunnel_key(struct action_util *au, FILE 
*f, struct rtattr *arg)
}
print_action_control(f, " ", parm->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", parm->index, parm->refcnt,
-   parm->bindcnt);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_uint(PRINT_ANY, "index", "\t index %u", parm->index);
+   print_int(PRINT_ANY, "ref", " ref %d", parm->refcnt);
+   print_int(PRINT_ANY, "bind", " bind %d", parm->bindcnt);
 
if (show_stats) {
if (tb[TCA_TUNNEL_KEY_TM]) {
@@ -302,7 +316,7 @@ static int print_tunnel_key(struct action_util *au, FILE 
*f, struct rtattr *arg)
}
}
 
-   fprintf(f, "\n ");
+   print_string(PRINT_FP, NULL, "%s", _SL_);
 
return 0;
 }
-- 
2.7.4

[PATCH iproute2-next 1/1] tc: jsonify skbedit action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_skbedit.c | 53 +
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c
index db5c64caf2ba..070280cea29e 100644
--- a/tc/m_skbedit.c
+++ b/tc/m_skbedit.c
@@ -168,9 +168,8 @@ static int print_skbedit(struct action_util *au, FILE *f, 
struct rtattr *arg)
struct rtattr *tb[TCA_SKBEDIT_MAX + 1];
 
SPRINT_BUF(b1);
-   __u32 *priority;
-   __u32 *mark;
-   __u16 *queue_mapping, *ptype;
+   __u32 priority;
+   __u16 ptype;
struct tc_skbedit *p = NULL;
 
if (arg == NULL)
@@ -179,43 +178,49 @@ static int print_skbedit(struct action_util *au, FILE *f, 
struct rtattr *arg)
parse_rtattr_nested(tb, TCA_SKBEDIT_MAX, arg);
 
if (tb[TCA_SKBEDIT_PARMS] == NULL) {
-   fprintf(f, "[NULL skbedit parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL skbedit parameters]");
return -1;
}
p = RTA_DATA(tb[TCA_SKBEDIT_PARMS]);
 
-   fprintf(f, " skbedit");
+   print_string(PRINT_ANY, "kind", "%s ", "skbedit");
 
if (tb[TCA_SKBEDIT_QUEUE_MAPPING] != NULL) {
-   queue_mapping = RTA_DATA(tb[TCA_SKBEDIT_QUEUE_MAPPING]);
-   fprintf(f, " queue_mapping %u", *queue_mapping);
+   print_uint(PRINT_ANY, "queue_mapping", "queue_mapping %u",
+  rta_getattr_u16(tb[TCA_SKBEDIT_QUEUE_MAPPING]));
}
if (tb[TCA_SKBEDIT_PRIORITY] != NULL) {
-   priority = RTA_DATA(tb[TCA_SKBEDIT_PRIORITY]);
-   fprintf(f, " priority %s", sprint_tc_classid(*priority, b1));
+   priority = rta_getattr_u32(tb[TCA_SKBEDIT_PRIORITY]);
+   print_string(PRINT_ANY, "priority", " priority %s",
+sprint_tc_classid(priority, b1));
}
if (tb[TCA_SKBEDIT_MARK] != NULL) {
-   mark = RTA_DATA(tb[TCA_SKBEDIT_MARK]);
-   fprintf(f, " mark %d", *mark);
+   print_uint(PRINT_ANY, "mark", " mark %u",
+  rta_getattr_u32(tb[TCA_SKBEDIT_MARK]));
}
if (tb[TCA_SKBEDIT_PTYPE] != NULL) {
-   ptype = RTA_DATA(tb[TCA_SKBEDIT_PTYPE]);
-   if (*ptype == PACKET_HOST)
-   fprintf(f, " ptype host");
-   else if (*ptype == PACKET_BROADCAST)
-   fprintf(f, " ptype broadcast");
-   else if (*ptype == PACKET_MULTICAST)
-   fprintf(f, " ptype multicast");
-   else if (*ptype == PACKET_OTHERHOST)
-   fprintf(f, " ptype otherhost");
+   ptype = rta_getattr_u16(tb[TCA_SKBEDIT_PTYPE]);
+   if (ptype == PACKET_HOST)
+   print_string(PRINT_ANY, "ptype", " %s", "ptype host");
+   else if (ptype == PACKET_BROADCAST)
+   print_string(PRINT_ANY, "ptype", " %s",
+"ptype broadcast");
+   else if (ptype == PACKET_MULTICAST)
+   print_string(PRINT_ANY, "ptype", " %s",
+"ptype multicast");
+   else if (ptype == PACKET_OTHERHOST)
+   print_string(PRINT_ANY, "ptype", " %s",
+"ptype otherhost");
else
-   fprintf(f, " ptype %d", *ptype);
+   print_uint(PRINT_ANY, "ptype", " %u", ptype);
}
 
print_action_control(f, " ", p->action, "");
 
-   fprintf(f, "\n\t index %u ref %d bind %d",
-   p->index, p->refcnt, p->bindcnt);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_uint(PRINT_ANY, "index", "\t index %u", p->index);
+   print_int(PRINT_ANY, "ref", " ref %d", p->refcnt);
+   print_int(PRINT_ANY, "bind", " bind %d", p->bindcnt);
 
if (show_stats) {
if (tb[TCA_SKBEDIT_TM]) {
@@ -225,7 +230,7 @@ static int print_skbedit(struct action_util *au, FILE *f, 
struct rtattr *arg)
}
}
 
-   fprintf(f, "\n ");
+   print_string(PRINT_FP, NULL, "%s", _SL_);
 
return 0;
 }
-- 
2.7.4

[PATCH iproute2-next 1/1] tc: jsonify connmark action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_connmark.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index bcce41391398..45e2d05f1a91 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -114,16 +114,20 @@ static int print_connmark(struct action_util *au, FILE 
*f, struct rtattr *arg)
 
parse_rtattr_nested(tb, TCA_CONNMARK_MAX, arg);
if (tb[TCA_CONNMARK_PARMS] == NULL) {
-   fprintf(f, "[NULL connmark parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL connmark 
parameters]");
return -1;
}
 
ci = RTA_DATA(tb[TCA_CONNMARK_PARMS]);
 
-   fprintf(f, " connmark zone %d", ci->zone);
-   print_action_control(f, " ", ci->action, "\n");
-   fprintf(f, "\t index %u ref %d bind %d", ci->index,
-   ci->refcnt, ci->bindcnt);
+   print_string(PRINT_ANY, "kind", "%s ", "connmark");
+   print_uint(PRINT_ANY, "zone", "zone %u", ci->zone);
+   print_action_control(f, " ", ci->action, "");
+
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_uint(PRINT_ANY, "index", "\t index %u", ci->index);
+   print_int(PRINT_ANY, "ref", " ref %d", ci->refcnt);
+   print_int(PRINT_ANY, "bind", " bind %d", ci->bindcnt);
 
if (show_stats) {
if (tb[TCA_CONNMARK_TM]) {
@@ -132,7 +136,7 @@ static int print_connmark(struct action_util *au, FILE *f, 
struct rtattr *arg)
print_tm(f, tm);
}
}
-   fprintf(f, "\n");
+   print_string(PRINT_FP, NULL, "%s", _SL_);
 
return 0;
 }
-- 
2.7.4

[PATCH iproute2-next 1/1] tc: jsonify sample action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_sample.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/tc/m_sample.c b/tc/m_sample.c
index 1e18c5154fe6..39a99246a8ea 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -149,23 +149,27 @@ static int print_sample(struct action_util *au, FILE *f, 
struct rtattr *arg)
 
if (!tb[TCA_SAMPLE_PARMS] || !tb[TCA_SAMPLE_RATE] ||
!tb[TCA_SAMPLE_PSAMPLE_GROUP]) {
-   fprintf(f, "[NULL sample parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL sample parameters]");
return -1;
}
p = RTA_DATA(tb[TCA_SAMPLE_PARMS]);
 
-   fprintf(f, "sample rate 1/%d group %d",
-   rta_getattr_u32(tb[TCA_SAMPLE_RATE]),
-   rta_getattr_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]));
+   print_string(PRINT_ANY, "kind", "%s ", "sample");
+   print_uint(PRINT_ANY, "rate", "rate 1/%u ",
+  rta_getattr_u32(tb[TCA_SAMPLE_RATE]));
+   print_uint(PRINT_ANY, "group", "group %u",
+  rta_getattr_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]));
 
if (tb[TCA_SAMPLE_TRUNC_SIZE])
-   fprintf(f, " trunc_size %d",
-   rta_getattr_u32(tb[TCA_SAMPLE_TRUNC_SIZE]));
+   print_uint(PRINT_ANY, "trunc_size", " trunc_size %u",
+  rta_getattr_u32(tb[TCA_SAMPLE_TRUNC_SIZE]));
 
print_action_control(f, " ", p->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", p->index, p->refcnt,
-   p->bindcnt);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
+   print_uint(PRINT_ANY, "index", "\t index %u", p->index);
+   print_int(PRINT_ANY, "ref", " ref %d", p->refcnt);
+   print_int(PRINT_ANY, "bind", " bind %d", p->bindcnt);
 
if (show_stats) {
if (tb[TCA_SAMPLE_TM]) {
@@ -174,7 +178,7 @@ static int print_sample(struct action_util *au, FILE *f, 
struct rtattr *arg)
print_tm(f, tm);
}
}
-   fprintf(f, "\n");
+   print_string(PRINT_FP, NULL, "%s", _SL_);
return 0;
 }
 
-- 
2.7.4

[PATCH iproute2-next 1/1] tc: support oneline mode in action generic printer functions

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_action.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 8891659ae15a..2f85d353279a 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -301,19 +301,21 @@ static int tc_print_one_action(FILE *f, struct rtattr 
*arg)
return err;
 
if (show_stats && tb[TCA_ACT_STATS]) {
-   print_string(PRINT_FP, NULL, "\tAction statistics:\n", NULL);
+   print_string(PRINT_FP, NULL, "\tAction statistics:", NULL);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
open_json_object("stats");
print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
close_json_object();
-   print_string(PRINT_FP, NULL, "\n", NULL);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
}
if (tb[TCA_ACT_COOKIE]) {
int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
char b1[strsz * 2 + 1];
 
-   print_string(PRINT_ANY, "cookie", "\tcookie %s\n",
+   print_string(PRINT_ANY, "cookie", "\tcookie %s",
 hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
   strsz, b1, sizeof(b1)));
+   print_string(PRINT_FP, NULL, "%s", _SL_);
}
 
return 0;
@@ -369,8 +371,9 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned 
short tot_acts)
for (i = 0; i <= tot_acts; i++) {
if (tb[i]) {
open_json_object(NULL);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
print_uint(PRINT_ANY, "order",
-  "\n\taction order %u: ", i);
+  "\taction order %u: ", i);
if (tc_print_one_action(f, tb[i]) < 0) {
print_string(PRINT_FP, NULL,
 "Error printing action\n", NULL);
@@ -410,6 +413,7 @@ int print_action(const struct sockaddr_nl *who,
open_json_object(NULL);
print_uint(PRINT_ANY, "total acts", "total acts %u",
   tot_acts ? *tot_acts : 0);
+   print_string(PRINT_FP, NULL, "%s", _SL_);
close_json_object();
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
-- 
2.7.4

[PATCH iproute2-next 1/1] tc: add online mode

[Roman Mashak]

Add initial support for oneline mode in tc; actions, filters and qdiscs
will be gradually updated in the follow-up patches.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 man/man8/tc.8 | 15 ++-
 tc/tc.c   |  8 +++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 3dc30ee489e5..840880fbdba6 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -95,7 +95,8 @@ tc \- show / manipulate traffic control settings
 \fB[ \fB-n\fR[\fIetns\fR] name \fB] \fR|
 \fB[ \fB-nm \fR| \fB-nam\fR[\fIes\fR] \fB] \fR|
 \fB[ \fR{ \fB-cf \fR| \fB-c\fR[\fIonf\fR] \fR} \fB[ filename ] \fB] \fR
-\fB[ -t\fR[imestamp\fR] \fB\] \fR| \fB[ -t\fR[short\fR] \fB]\fR }
+\fB[ -t\fR[imestamp\fR] \fB\] \fR| \fB[ -t\fR[short\fR] \fR| \fB[
+-o\fR[neline\fR] \fB]\fR }
 
 .ti 8
 .IR FORMAT " := {"
@@ -649,6 +650,18 @@ don't terminate tc on errors in batch mode.
 If there were any errors during execution of the commands, the application 
return code will be non zero.
 
 .TP
+.BR "\-o" , " \-oneline"
+output each record on a single line, replacing line feeds
+with the
+.B '\e'
+character. This is convenient when you want to count records
+with
+.BR wc (1)
+or to
+.BR grep (1)
+the output.
+
+.TP
 .BR "\-n" , " \-net" , " \-netns " 
 switches
 .B tc
diff --git a/tc/tc.c b/tc/tc.c
index a31f075d1ffe..68475c156057 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -42,6 +42,8 @@ int force;
 bool use_names;
 int json;
 int color;
+int oneline;
+const char *_SL_;
 
 static char *conf_file;
 
@@ -191,7 +193,7 @@ static void usage(void)
"where  OBJECT := { qdisc | class | filter | action | monitor | 
exec }\n"
"   OPTIONS := { -s[tatistics] | -d[etails] | -r[aw] | 
-b[atch] [filename] | -n[etns] name |\n"
"-nm | -nam[es] | { -cf | -conf } path } 
|\n"
-   "-j[son] -p[retty] -c[olor]\n");
+   "-o[neline] -j[son] -p[retty] -c[olor]\n");
 }
 
 static int do_cmd(int argc, char **argv, void *buf, size_t buflen)
@@ -487,6 +489,8 @@ int main(int argc, char **argv)
++timestamp_short;
} else if (matches(argv[1], "-json") == 0) {
++json;
+   } else if (matches(argv[1], "-oneline") == 0) {
+   ++oneline;
} else {
fprintf(stderr, "Option \"%s\" is unknown, try \"tc 
-help\".\n", argv[1]);
return -1;
@@ -494,6 +498,8 @@ int main(int argc, char **argv)
argc--; argv++;
}
 
+   _SL_ = oneline ? "\\" : "\n";
+
if (color & !json)
enable_color();
 
-- 
2.7.4

[PATCH net-next 1/1] tc-testing: add connmark action tests

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/connmark.json  | 291 +
 1 file changed, 291 insertions(+)
 create mode 100644 
tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json
new file mode 100644
index ..70952bd98ff9
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/connmark.json
@@ -0,0 +1,291 @@
+[
+{
+"id": "2002",
+"name": "Add valid connmark action with defaults",
+"category": [
+"actions",
+"connmark"
+],
+"setup": [
+[
+"$TC actions flush action connmark",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action connmark",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action connmark",
+"matchPattern": "action order [0-9]+:  connmark zone 0 pipe",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action connmark"
+]
+},
+{
+"id": "56a5",
+"name": "Add valid connmark action with control pass",
+"category": [
+"actions",
+"connmark"
+],
+"setup": [
+[
+"$TC actions flush action connmark",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action connmark pass index 1",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action connmark index 1",
+"matchPattern": "action order [0-9]+:  connmark zone 0 pass.*index 1 
ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action connmark"
+]
+},
+{
+"id": "7c66",
+"name": "Add valid connmark action with control drop",
+"category": [
+"actions",
+"connmark"
+],
+"setup": [
+[
+"$TC actions flush action connmark",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action connmark drop index 100",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action connmark index 100",
+"matchPattern": "action order [0-9]+:  connmark zone 0 drop.*index 100 
ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action connmark"
+]
+},
+{
+"id": "a913",
+"name": "Add valid connmark action with control pipe",
+"category": [
+"actions",
+"connmark"
+],
+"setup": [
+[
+"$TC actions flush action connmark",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action connmark pipe index 455",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action connmark index 455",
+"matchPattern": "action order [0-9]+:  connmark zone 0 pipe.*index 455 
ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action connmark"
+]
+},
+{
+"id": "bdd8",
+"name": "Add valid connmark action with control reclassify",
+"category": [
+"actions",
+"connmark"
+],
+"setup": [
+[
+"$TC actions flush action connmark",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action connmark reclassify index 7",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action connmark",
+"matchPattern": &q

[PATCH iproute2-next 1/1] tc: enable json output for actions

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_action.c | 23 ++-
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 6c3049c7db88..8891659ae15a 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -364,6 +364,7 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned 
short tot_acts)
if (tab_flush && NULL != tb[0]  && NULL == tb[1])
return tc_print_action_flush(f, tb[0]);
 
+   open_json_object(NULL);
open_json_array(PRINT_JSON, "actions");
for (i = 0; i <= tot_acts; i++) {
if (tb[i]) {
@@ -379,6 +380,7 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned 
short tot_acts)
 
}
close_json_array(PRINT_JSON, NULL);
+   close_json_object();
 
return 0;
 }
@@ -405,7 +407,10 @@ int print_action(const struct sockaddr_nl *who,
if (tb[TCA_ROOT_COUNT])
tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
 
-   fprintf(fp, "total acts %d\n", tot_acts ? *tot_acts:0);
+   open_json_object(NULL);
+   print_uint(PRINT_ANY, "total acts", "total acts %u",
+  tot_acts ? *tot_acts : 0);
+   close_json_object();
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
fprintf(stderr, "print_action: NULL kind\n");
@@ -531,10 +536,16 @@ static int tc_action_gd(int cmd, unsigned int flags,
return 1;
}
 
-   if (cmd == RTM_GETACTION && print_action(NULL, ans, stdout) < 0) {
-   fprintf(stderr, "Dump terminated\n");
-   free(ans);
-   return 1;
+   if (cmd == RTM_GETACTION) {
+   new_json_obj(json);
+   ret = print_action(NULL, ans, stdout);
+   if (ret < 0) {
+   fprintf(stderr, "Dump terminated\n");
+   free(ans);
+   delete_json_obj();
+   return 1;
+   }
+   delete_json_obj();
}
free(ans);
 
@@ -675,7 +686,9 @@ static int tc_act_list_or_flush(int *argc_p, char 
***argv_p, int event)
perror("Cannot send dump request");
return 1;
}
+   new_json_obj(json);
ret = rtnl_dump_filter(, print_action, stdout);
+   delete_json_obj();
}
 
if (event == RTM_DELACTION) {
-- 
2.7.4

Re: [PATCH v3 iproute2 1/1] tc: fix conversion types when printing actions unsigned values

[Roman Mashak]

Stephen Hemminger <step...@networkplumber.org> writes:

> On Mon, 19 Mar 2018 17:05:41 -0400
> Roman Mashak <m...@mojatatu.com> wrote:
>
>> diff --git a/tc/m_gact.c b/tc/m_gact.c
>> index 16c4413f4217..52022415db48 100644
>> --- a/tc/m_gact.c
>> +++ b/tc/m_gact.c
>> @@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct 
>> rtattr *arg)
>>  print_string(PRINT_ANY, "random_type", "\n\t random type %s",
>>   prob_n2a(pp->ptype));
>>  print_action_control(f, " ", pp->paction, " ");
>> -print_int(PRINT_ANY, "val", "val %d", pp->pval);
>> +print_int(PRINT_ANY, "val", "val %u", pp->pval);
>>  close_json_object();
>
> This needs to be print_uint in order to work correctly with json output.
>
> Also, please consider doing json on all the match types in a later patch
> for net-next.

Did you mean iproute2-next?

Re: [PATCH iproute2 1/1] tc: print index, refcnt & bindcnt for nat action

[Roman Mashak]

Stephen Hemminger <step...@networkplumber.org> writes:

> On Tue, 20 Mar 2018 14:21:47 -0400
> Roman Mashak <m...@mojatatu.com> wrote:
>
>> Signed-off-by: Roman Mashak <m...@mojatatu.com>
>> ---
>>  tc/m_nat.c | 5 +
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/tc/m_nat.c b/tc/m_nat.c
>> index 1e4ff51fe75a..f6e373957c1b 100644
>> --- a/tc/m_nat.c
>> +++ b/tc/m_nat.c
>> @@ -169,6 +169,9 @@ print_nat(struct action_util *au, FILE * f, struct 
>> rtattr *arg)
>>  format_host_r(AF_INET, 4, >new_addr, buf2, sizeof(buf2)));
>>  print_action_control(f, " ", sel->action, "");
>>  
>> +fprintf(f, "\n\t index %u ref %d bind %d",
>> +sel->index, sel->refcnt, sel->bindcnt);
>> +
>>  if (show_stats) {
>>  if (tb[TCA_NAT_TM]) {
>>  struct tcf_t *tm = RTA_DATA(tb[TCA_NAT_TM]);
>> @@ -177,6 +180,8 @@ print_nat(struct action_util *au, FILE * f, struct 
>> rtattr *arg)
>>  }
>>  }
>>  
>> +fprintf(f, "\n");
>> +
>>  return 0;
>>  }
>
> Rather than printing newline all the time, you need to use _SL_ to keep the 
> optional
> oneline output format.
>
> I.e
>   fprintf(f, "%s\t index %u ref %d bind %d",
>   _SL_, sel->index, sel->refcnt, sel->bindcnt);

tc currently doesn't support oneline mode, so I'll have to add it first,
then update all actions to use the oneliner.

[RESEND PATCH net-next 1/1] tc-testing: updated police, mirred, skbedit and skbmod with more tests

[Roman Mashak]

Added extra test cases for control actions (reclassify, pipe etc.),
cookies, max index value and police args sanity check.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/mirred.json| 192 +
 .../tc-testing/tc-tests/actions/police.json| 144 
 .../tc-testing/tc-tests/actions/skbedit.json   | 168 ++
 .../tc-testing/tc-tests/actions/skbmod.json|  26 ++-
 4 files changed, 529 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
index 0fcccf18399b..443c9b3c8664 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
@@ -171,6 +171,198 @@
 ]
 },
 {
+"id": "8917",
+"name": "Add mirred mirror action with control pass",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
pass index 1",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 1",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) pass.*index 1 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "1054",
+"name": "Add mirred mirror action with control pipe",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
pipe index 15",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 15",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) pipe.*index 15 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "9887",
+"name": "Add mirred mirror action with control continue",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
continue index 15",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 15",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) continue.*index 15 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "e4aa",
+"name": "Add mirred mirror action with control reclassify",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
reclassify index 150",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 150",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) reclassify.*index 150 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "ece9",
+"name": "Add mirred mirror action with control drop",
+"category": [
+ 

Re: [PATCH iproute2] ss: Fix rendering of continuous output (-E, --events)

[Roman Mashak]

Stefano Brivio <sbri...@redhat.com> writes:

> Roman Mashak reported that ss currently shows no output when it
> should continuously report information about terminated sockets
> (-E, --events switch).
>
> This happens because I missed this case in 691bd854bf4a ("ss:
> Buffer raw fields first, then render them as a table") and the
> rendering function is simply not called.
>
> To fix this, we need to:
>
> - call render() every time we need to display new socket events
>   from generic_show_sock(), which is only used to follow events.
>   Always call it even if specific socket display functions
>   return errors to ensure we clean up buffers
>
> - get the screen width every time we have new events to display,
>   thus factor out getting the screen width from main() into a
>   function we'll call whenever we calculate columns width
>
> - reset the current field pointer after rendering, more output
>   might come after render() is called
>
> Reported-by: Roman Mashak <m...@mojatatu.com>
> Fixes: 691bd854bf4a ("ss: Buffer raw fields first, then render them as a 
> table")
> Signed-off-by: Stefano Brivio <sbri...@redhat.com>

Thanks Stefano.

Tested-by: Roman Mashak <m...@mojatatu.com>

[PATCH net-next 1/1] net sched actions: merge event notification routines

[Roman Mashak]

Collapse tca_get_notify(), tca_add_notify() and tca_del_notify() in a
single function since they repeat the same code pattern.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_api.c | 111 
 1 file changed, 33 insertions(+), 78 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 57cf37145282..5b04184fb525 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -895,24 +895,41 @@ static int tca_get_fill(struct sk_buff *skb, struct 
list_head *actions,
return -1;
 }
 
-static int
-tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
-  struct list_head *actions, int event,
-  struct netlink_ext_ack *extack)
+static int tca_notify(struct net *net, struct nlmsghdr *n,
+ struct list_head *actions, u32 portid, int event,
+ size_t attr_size, struct netlink_ext_ack *extack)
 {
struct sk_buff *skb;
+   int err;
 
-   skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
+   GFP_KERNEL);
if (!skb)
return -ENOBUFS;
-   if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
-0, 0) <= 0) {
-   NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while 
adding TC action");
+
+   if (tca_get_fill(skb, actions, portid, n->nlmsg_seq,
+event == RTM_NEWACTION ? n->nlmsg_flags : 0,
+event, 0,
+event == RTM_DELACTION ? 1 : 0) <= 0) {
+   NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action 
attributes in event message");
kfree_skb(skb);
return -EINVAL;
}
 
-   return rtnl_unicast(skb, net, portid);
+   if (event == RTM_GETACTION) {
+   return rtnl_unicast(skb, net, portid);
+   } else if (event == RTM_DELACTION) {
+   /* now do the delete */
+   err = tcf_action_destroy(actions, 0);
+   if (err < 0) {
+   NL_SET_ERR_MSG(extack, "Failed to delete TC action");
+   kfree_skb(skb);
+   return err;
+   }
+   }
+   err = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+n->nlmsg_flags & NLM_F_ECHO);
+   return err > 0 ? 0 : err;
 }
 
 static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
@@ -1034,40 +1051,6 @@ static int tca_action_flush(struct net *net, struct 
nlattr *nla,
 }
 
 static int
-tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
-{
-   int ret;
-   struct sk_buff *skb;
-
-   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
-   GFP_KERNEL);
-   if (!skb)
-   return -ENOBUFS;
-
-   if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
-0, 1) <= 0) {
-   NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action 
attributes");
-   kfree_skb(skb);
-   return -EINVAL;
-   }
-
-   /* now do the delete */
-   ret = tcf_action_destroy(actions, 0);
-   if (ret < 0) {
-   NL_SET_ERR_MSG(extack, "Failed to delete TC action");
-   kfree_skb(skb);
-   return ret;
-   }
-
-   ret = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
-n->nlmsg_flags & NLM_F_ECHO);
-   if (ret > 0)
-   return 0;
-   return ret;
-}
-
-static int
 tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
  u32 portid, int event, struct netlink_ext_ack *extack)
 {
@@ -1102,46 +1085,17 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
 
attr_size = tcf_action_full_attrs_size(attr_size);
 
-   if (event == RTM_GETACTION)
-   ret = tcf_get_notify(net, portid, n, , event, extack);
-   else { /* delete */
-   ret = tcf_del_notify(net, n, , portid, attr_size, 
extack);
-   if (ret)
-   goto err;
-   return ret;
-   }
+   ret = tca_notify(net, n, , portid, event, attr_size, extack);
+   if (ret)
+   goto err;
+   return ret;
+
 err:
if (event != RTM_GETACTION)
tcf_action_destroy(, 0);
return ret;
 }
 
-static int
-tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
-{
-   struct sk_buff *skb;
-   int err = 0;
-
-   

Re: [RESEND PATCH net-next 1/1] tc-testing: updated police, mirred, skbedit and skbmod with more tests

[Roman Mashak]

David Miller <da...@davemloft.net> writes:

> From: Roman Mashak <m...@mojatatu.com>
> Date: Thu, 22 Mar 2018 08:23:22 -0400
>
>> diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json 
>> b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
>> index 90bba48c3f07..8aa5a88ccb19 100644
>> --- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
>> +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
>  ...
>>  ]
>>  }
>> -]
>> +]
>> \ No newline at end of file
>> -- 
>> 2.7.4
>
> Please fix this.

The patch updates police.json, mirred.json, skbedit.json and
skbmod.json files that initially had no newline on the end.

[RESEND PATCH net-next 1/1] tc-testing: updated police, mirred, skbedit and skbmod with more tests

[Roman Mashak]

Added extra test cases for control actions (reclassify, pipe etc.),
cookies, max index value and police args sanity check.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/mirred.json| 192 +
 .../tc-testing/tc-tests/actions/police.json| 144 
 .../tc-testing/tc-tests/actions/skbedit.json   | 168 ++
 .../tc-testing/tc-tests/actions/skbmod.json|  26 ++-
 4 files changed, 529 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
index 0fcccf18399b..443c9b3c8664 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
@@ -171,6 +171,198 @@
 ]
 },
 {
+"id": "8917",
+"name": "Add mirred mirror action with control pass",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
pass index 1",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 1",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) pass.*index 1 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "1054",
+"name": "Add mirred mirror action with control pipe",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
pipe index 15",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 15",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) pipe.*index 15 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "9887",
+"name": "Add mirred mirror action with control continue",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
continue index 15",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 15",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) continue.*index 15 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "e4aa",
+"name": "Add mirred mirror action with control reclassify",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
reclassify index 150",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 150",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) reclassify.*index 150 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "ece9",
+"name": "Add mirred mirror action with control drop",
+"category": [
+ 

[PATCH net-next 1/1] tc-testing: updated police, mirred, skbedit and skbmod with more tests

[Roman Mashak]

Added extra test cases for control actions (reclassify, pipe etc.),
cookies, max index value and police args sanity check.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/mirred.json| 192 +
 .../tc-testing/tc-tests/actions/police.json| 144 
 .../tc-testing/tc-tests/actions/skbedit.json   | 168 ++
 .../tc-testing/tc-tests/actions/skbmod.json|  26 ++-
 4 files changed, 529 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
index 0fcccf18399b..443c9b3c8664 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
@@ -171,6 +171,198 @@
 ]
 },
 {
+"id": "8917",
+"name": "Add mirred mirror action with control pass",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
pass index 1",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 1",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) pass.*index 1 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "1054",
+"name": "Add mirred mirror action with control pipe",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
pipe index 15",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 15",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) pipe.*index 15 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "9887",
+"name": "Add mirred mirror action with control continue",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
continue index 15",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 15",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) continue.*index 15 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "e4aa",
+"name": "Add mirred mirror action with control reclassify",
+"category": [
+"actions",
+"mirred"
+],
+"setup": [
+[
+"$TC actions flush action mirred",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo 
reclassify index 150",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action mirred index 150",
+"matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to 
device lo\\) reclassify.*index 150 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action mirred"
+]
+},
+{
+"id": "ece9",
+"name": "Add mirred mirror action with control drop",
+"category": [
+ 

[PATCH iproute2 1/1] tc: print index, refcnt & bindcnt for nat action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_nat.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tc/m_nat.c b/tc/m_nat.c
index 1e4ff51fe75a..f6e373957c1b 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -169,6 +169,9 @@ print_nat(struct action_util *au, FILE * f, struct rtattr 
*arg)
format_host_r(AF_INET, 4, >new_addr, buf2, sizeof(buf2)));
print_action_control(f, " ", sel->action, "");
 
+   fprintf(f, "\n\t index %u ref %d bind %d",
+   sel->index, sel->refcnt, sel->bindcnt);
+
if (show_stats) {
if (tb[TCA_NAT_TM]) {
struct tcf_t *tm = RTA_DATA(tb[TCA_NAT_TM]);
@@ -177,6 +180,8 @@ print_nat(struct action_util *au, FILE * f, struct rtattr 
*arg)
}
}
 
+   fprintf(f, "\n");
+
return 0;
 }
 
-- 
2.7.4

[PATCH iproute2 1/1] tc: print actual action for connmark action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_connmark.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index 37d718541549..d5b140cbb7bd 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -121,7 +121,8 @@ static int print_connmark(struct action_util *au, FILE *f, 
struct rtattr *arg)
 
ci = RTA_DATA(tb[TCA_CONNMARK_PARMS]);
 
-   fprintf(f, " connmark zone %d\n", ci->zone);
+   fprintf(f, " connmark zone %d", ci->zone);
+   print_action_control(f, " ", ci->action, "\n");
fprintf(f, "\t index %u ref %d bind %d", ci->index,
ci->refcnt, ci->bindcnt);
 
-- 
2.7.4

[PATCH v3 iproute2 1/1] tc: fix conversion types when printing actions unsigned values

[Roman Mashak]

v3:
   fixed conversion in connmark missed out in first version
v2:
   fixed coding style

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_action.c | 2 +-
 tc/m_connmark.c   | 2 +-
 tc/m_gact.c   | 2 +-
 tc/m_ife.c| 2 +-
 tc/m_pedit.c  | 2 +-
 tc/m_sample.c | 6 +++---
 tc/m_tunnel_key.c | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 148f1372d414..244c1ec00f28 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -408,7 +408,7 @@ int print_action(const struct sockaddr_nl *who,
if (tb[TCA_ROOT_COUNT])
tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
 
-   fprintf(fp, "total acts %d\n", tot_acts ? *tot_acts:0);
+   fprintf(fp, "total acts %u\n", tot_acts ? *tot_acts : 0);
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
fprintf(stderr, "print_action: NULL kind\n");
diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index 37d718541549..7c4ba7ae5301 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -121,7 +121,7 @@ static int print_connmark(struct action_util *au, FILE *f, 
struct rtattr *arg)
 
ci = RTA_DATA(tb[TCA_CONNMARK_PARMS]);
 
-   fprintf(f, " connmark zone %d\n", ci->zone);
+   fprintf(f, " connmark zone %u\n", ci->zone);
fprintf(f, "\t index %u ref %d bind %d", ci->index,
ci->refcnt, ci->bindcnt);
 
diff --git a/tc/m_gact.c b/tc/m_gact.c
index 16c4413f4217..52022415db48 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr 
*arg)
print_string(PRINT_ANY, "random_type", "\n\t random type %s",
 prob_n2a(pp->ptype));
print_action_control(f, " ", pp->paction, " ");
-   print_int(PRINT_ANY, "val", "val %d", pp->pval);
+   print_int(PRINT_ANY, "val", "val %u", pp->pval);
close_json_object();
 #endif
print_uint(PRINT_ANY, "index", "\n\t index %u", p->index);
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 205efc9f1d9a..e1dbd3a79649 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -280,7 +280,7 @@ static int print_ife(struct action_util *au, FILE *f, 
struct rtattr *arg)
if (len) {
mtcindex =

rta_getattr_u16(metalist[IFE_META_TCINDEX]);
-   fprintf(f, "use tcindex %d ", mtcindex);
+   fprintf(f, "use tcindex %u ", mtcindex);
} else
fprintf(f, "allow tcindex ");
}
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 26549eeea899..151dfe1a230a 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -817,7 +817,7 @@ int print_pedit(struct action_util *au, FILE *f, struct 
rtattr *arg)
(unsigned int)ntohl(key->mask));
}
} else {
-   fprintf(f, "\npedit %x keys %d is not LEGIT", sel->index,
+   fprintf(f, "\npedit %x keys %u is not LEGIT", sel->index,
sel->nkeys);
}
 
diff --git a/tc/m_sample.c b/tc/m_sample.c
index 01763cb4c356..d42a6a327965 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -155,17 +155,17 @@ static int print_sample(struct action_util *au, FILE *f, 
struct rtattr *arg)
}
p = RTA_DATA(tb[TCA_SAMPLE_PARMS]);
 
-   fprintf(f, "sample rate 1/%d group %d",
+   fprintf(f, "sample rate 1/%u group %u",
rta_getattr_u32(tb[TCA_SAMPLE_RATE]),
rta_getattr_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]));
 
if (tb[TCA_SAMPLE_TRUNC_SIZE])
-   fprintf(f, " trunc_size %d",
+   fprintf(f, " trunc_size %u",
rta_getattr_u32(tb[TCA_SAMPLE_TRUNC_SIZE]));
 
print_action_control(f, " ", p->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", p->index, p->refcnt,
+   fprintf(f, "\n\tindex %u ref %d bind %d", p->index, p->refcnt,
p->bindcnt);
 
if (show_stats) {
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index 1cdd03560c35..dd8f8e8c635b 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -292,7 +292,7 @@ static int print_tunnel_key(struct action_util *au, FILE 
*f, struct rtattr *arg)
}
print_action_control(f, " ", parm->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", parm->index, parm->refcnt,
+   fprintf(f, "\n\tindex %u ref %d bind %d", parm->index, parm->refcnt,
parm->bindcnt);
 
if (show_stats) {
-- 
2.7.4

Re: [PATCH v2 iproute2 1/1] tc: fix conversion types when printing actions unsigned values

[Roman Mashak]

Please disregard this patch, I will send v3.

On Mon, Mar 19, 2018 at 2:13 PM, Roman Mashak <m...@mojatatu.com> wrote:
> v2:
>fixed coding style
>
> Signed-off-by: Roman Mashak <m...@mojatatu.com>
> ---
>  tc/m_action.c | 2 +-
>  tc/m_gact.c   | 2 +-
>  tc/m_ife.c| 2 +-
>  tc/m_pedit.c  | 2 +-
>  tc/m_sample.c | 6 +++---
>  tc/m_tunnel_key.c | 2 +-
>  6 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/tc/m_action.c b/tc/m_action.c
> index 148f1372d414..244c1ec00f28 100644
> --- a/tc/m_action.c
> +++ b/tc/m_action.c
> @@ -408,7 +408,7 @@ int print_action(const struct sockaddr_nl *who,
> if (tb[TCA_ROOT_COUNT])
> tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
>
> -   fprintf(fp, "total acts %d\n", tot_acts ? *tot_acts:0);
> +   fprintf(fp, "total acts %u\n", tot_acts ? *tot_acts : 0);
> if (tb[TCA_ACT_TAB] == NULL) {
> if (n->nlmsg_type != RTM_GETACTION)
> fprintf(stderr, "print_action: NULL kind\n");
> diff --git a/tc/m_gact.c b/tc/m_gact.c
> index 16c4413f4217..52022415db48 100644
> --- a/tc/m_gact.c
> +++ b/tc/m_gact.c
> @@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr 
> *arg)
> print_string(PRINT_ANY, "random_type", "\n\t random type %s",
>  prob_n2a(pp->ptype));
> print_action_control(f, " ", pp->paction, " ");
> -   print_int(PRINT_ANY, "val", "val %d", pp->pval);
> +   print_int(PRINT_ANY, "val", "val %u", pp->pval);
> close_json_object();
>  #endif
> print_uint(PRINT_ANY, "index", "\n\t index %u", p->index);
> diff --git a/tc/m_ife.c b/tc/m_ife.c
> index 205efc9f1d9a..e1dbd3a79649 100644
> --- a/tc/m_ife.c
> +++ b/tc/m_ife.c
> @@ -280,7 +280,7 @@ static int print_ife(struct action_util *au, FILE *f, 
> struct rtattr *arg)
> if (len) {
> mtcindex =
> 
> rta_getattr_u16(metalist[IFE_META_TCINDEX]);
> -   fprintf(f, "use tcindex %d ", mtcindex);
> +   fprintf(f, "use tcindex %u ", mtcindex);
> } else
> fprintf(f, "allow tcindex ");
> }
> diff --git a/tc/m_pedit.c b/tc/m_pedit.c
> index 26549eeea899..151dfe1a230a 100644
> --- a/tc/m_pedit.c
> +++ b/tc/m_pedit.c
> @@ -817,7 +817,7 @@ int print_pedit(struct action_util *au, FILE *f, struct 
> rtattr *arg)
> (unsigned int)ntohl(key->mask));
> }
> } else {
> -   fprintf(f, "\npedit %x keys %d is not LEGIT", sel->index,
> +   fprintf(f, "\npedit %x keys %u is not LEGIT", sel->index,
> sel->nkeys);
> }
>
> diff --git a/tc/m_sample.c b/tc/m_sample.c
> index 01763cb4c356..d42a6a327965 100644
> --- a/tc/m_sample.c
> +++ b/tc/m_sample.c
> @@ -155,17 +155,17 @@ static int print_sample(struct action_util *au, FILE 
> *f, struct rtattr *arg)
> }
> p = RTA_DATA(tb[TCA_SAMPLE_PARMS]);
>
> -   fprintf(f, "sample rate 1/%d group %d",
> +   fprintf(f, "sample rate 1/%u group %u",
> rta_getattr_u32(tb[TCA_SAMPLE_RATE]),
> rta_getattr_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]));
>
> if (tb[TCA_SAMPLE_TRUNC_SIZE])
> -   fprintf(f, " trunc_size %d",
> +   fprintf(f, " trunc_size %u",
> rta_getattr_u32(tb[TCA_SAMPLE_TRUNC_SIZE]));
>
> print_action_control(f, " ", p->action, "");
>
> -   fprintf(f, "\n\tindex %d ref %d bind %d", p->index, p->refcnt,
> +   fprintf(f, "\n\tindex %u ref %d bind %d", p->index, p->refcnt,
> p->bindcnt);
>
>     if (show_stats) {
> diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
> index 1cdd03560c35..dd8f8e8c635b 100644
> --- a/tc/m_tunnel_key.c
> +++ b/tc/m_tunnel_key.c
> @@ -292,7 +292,7 @@ static int print_tunnel_key(struct action_util *au, FILE 
> *f, struct rtattr *arg)
> }
> print_action_control(f, " ", parm->action, "");
>
> -   fprintf(f, "\n\tindex %d ref %d bind %d", parm->index, parm->refcnt,
> +   fprintf(f, "\n\tindex %u ref %d bind %d", parm->index, parm->refcnt,
> parm->bindcnt);
>
> if (show_stats) {
> --
> 2.7.4
>



-- 
Roman Mashak

[PATCH v2 iproute2 1/1] tc: fix conversion types when printing actions unsigned values

[Roman Mashak]

v2:
   fixed coding style

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_action.c | 2 +-
 tc/m_gact.c   | 2 +-
 tc/m_ife.c| 2 +-
 tc/m_pedit.c  | 2 +-
 tc/m_sample.c | 6 +++---
 tc/m_tunnel_key.c | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 148f1372d414..244c1ec00f28 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -408,7 +408,7 @@ int print_action(const struct sockaddr_nl *who,
if (tb[TCA_ROOT_COUNT])
tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
 
-   fprintf(fp, "total acts %d\n", tot_acts ? *tot_acts:0);
+   fprintf(fp, "total acts %u\n", tot_acts ? *tot_acts : 0);
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
fprintf(stderr, "print_action: NULL kind\n");
diff --git a/tc/m_gact.c b/tc/m_gact.c
index 16c4413f4217..52022415db48 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr 
*arg)
print_string(PRINT_ANY, "random_type", "\n\t random type %s",
 prob_n2a(pp->ptype));
print_action_control(f, " ", pp->paction, " ");
-   print_int(PRINT_ANY, "val", "val %d", pp->pval);
+   print_int(PRINT_ANY, "val", "val %u", pp->pval);
close_json_object();
 #endif
print_uint(PRINT_ANY, "index", "\n\t index %u", p->index);
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 205efc9f1d9a..e1dbd3a79649 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -280,7 +280,7 @@ static int print_ife(struct action_util *au, FILE *f, 
struct rtattr *arg)
if (len) {
mtcindex =

rta_getattr_u16(metalist[IFE_META_TCINDEX]);
-   fprintf(f, "use tcindex %d ", mtcindex);
+   fprintf(f, "use tcindex %u ", mtcindex);
} else
fprintf(f, "allow tcindex ");
}
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 26549eeea899..151dfe1a230a 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -817,7 +817,7 @@ int print_pedit(struct action_util *au, FILE *f, struct 
rtattr *arg)
(unsigned int)ntohl(key->mask));
}
} else {
-   fprintf(f, "\npedit %x keys %d is not LEGIT", sel->index,
+   fprintf(f, "\npedit %x keys %u is not LEGIT", sel->index,
sel->nkeys);
}
 
diff --git a/tc/m_sample.c b/tc/m_sample.c
index 01763cb4c356..d42a6a327965 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -155,17 +155,17 @@ static int print_sample(struct action_util *au, FILE *f, 
struct rtattr *arg)
}
p = RTA_DATA(tb[TCA_SAMPLE_PARMS]);
 
-   fprintf(f, "sample rate 1/%d group %d",
+   fprintf(f, "sample rate 1/%u group %u",
rta_getattr_u32(tb[TCA_SAMPLE_RATE]),
rta_getattr_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]));
 
if (tb[TCA_SAMPLE_TRUNC_SIZE])
-   fprintf(f, " trunc_size %d",
+   fprintf(f, " trunc_size %u",
rta_getattr_u32(tb[TCA_SAMPLE_TRUNC_SIZE]));
 
print_action_control(f, " ", p->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", p->index, p->refcnt,
+   fprintf(f, "\n\tindex %u ref %d bind %d", p->index, p->refcnt,
p->bindcnt);
 
if (show_stats) {
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index 1cdd03560c35..dd8f8e8c635b 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -292,7 +292,7 @@ static int print_tunnel_key(struct action_util *au, FILE 
*f, struct rtattr *arg)
}
print_action_control(f, " ", parm->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", parm->index, parm->refcnt,
+   fprintf(f, "\n\tindex %u ref %d bind %d", parm->index, parm->refcnt,
parm->bindcnt);
 
if (show_stats) {
-- 
2.7.4

Re: [PATCH iproute2 1/1] tc: fix conversion types when printing actions unsigned values

[Roman Mashak]

Stephen Hemminger <step...@networkplumber.org> writes:

> On Mon, 19 Mar 2018 13:50:07 -0400
> Roman Mashak <m...@mojatatu.com> wrote:
>
>> Signed-off-by: Roman Mashak <m...@mojatatu.com>
>> ---
>>  tc/m_action.c | 2 +-
>>  tc/m_gact.c   | 2 +-
>>  tc/m_ife.c| 2 +-
>>  tc/m_pedit.c  | 2 +-
>>  tc/m_sample.c | 6 +++---
>>  tc/m_tunnel_key.c | 2 +-
>>  6 files changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/tc/m_action.c b/tc/m_action.c
>> index 148f1372d414..85c9d44c7e50 100644
>> --- a/tc/m_action.c
>> +++ b/tc/m_action.c
>> @@ -408,7 +408,7 @@ int print_action(const struct sockaddr_nl *who,
>>  if (tb[TCA_ROOT_COUNT])
>>  tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
>>  
>> -fprintf(fp, "total acts %d\n", tot_acts ? *tot_acts:0);
>> +fprintf(fp, "total acts %u\n", tot_acts ? *tot_acts:0);
>
> Please add spaces around : in trigraph.
>
> When fixing code, it has to pass style checkers.

Thanks, I will send v2.

Re: [PATCH net-next] tc-testing: add selftests for 'bpf' action

[Roman Mashak]

Davide Caratti  writes:

> Test d959: Add cBPF action with valid bytecode
> Test f84a: Add cBPF action with invalid bytecode
> Test e939: Add eBPF action with valid object-file
> Test d819: Replace cBPF bytecode and action control
> Test 6ae3: Delete cBPF action
> Test 3e0d: List cBPF actions
> Test 55ce: Flush BPF actions
> Test ccc3: Add cBPF action with duplicate index
>
> Signed-off-by: Davide Caratti 
> ---
>  .../selftests/tc-testing/tc-tests/actions/bpf.json | 215 
> +
>  1 file changed, 215 insertions(+)
>  create mode 100644 
> tools/testing/selftests/tc-testing/tc-tests/actions/bpf.json
>
> diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/bpf.json 
> b/tools/testing/selftests/tc-testing/tc-tests/actions/bpf.json
> new file mode 100644
> index ..0295a63dd0c8

[...]

Davide, thank you for efforts. Please also add a test for testing max
index value (since it is u32 type) and a test with cookie (every action
may have optional 128-bit value, that carries some user state and not
intepreted by the kernel).

[PATCH iproute2 1/1] tc: fix conversion types when printing actions unsigned values

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_action.c | 2 +-
 tc/m_gact.c   | 2 +-
 tc/m_ife.c| 2 +-
 tc/m_pedit.c  | 2 +-
 tc/m_sample.c | 6 +++---
 tc/m_tunnel_key.c | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 148f1372d414..85c9d44c7e50 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -408,7 +408,7 @@ int print_action(const struct sockaddr_nl *who,
if (tb[TCA_ROOT_COUNT])
tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
 
-   fprintf(fp, "total acts %d\n", tot_acts ? *tot_acts:0);
+   fprintf(fp, "total acts %u\n", tot_acts ? *tot_acts:0);
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
fprintf(stderr, "print_action: NULL kind\n");
diff --git a/tc/m_gact.c b/tc/m_gact.c
index 16c4413f4217..52022415db48 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr 
*arg)
print_string(PRINT_ANY, "random_type", "\n\t random type %s",
 prob_n2a(pp->ptype));
print_action_control(f, " ", pp->paction, " ");
-   print_int(PRINT_ANY, "val", "val %d", pp->pval);
+   print_int(PRINT_ANY, "val", "val %u", pp->pval);
close_json_object();
 #endif
print_uint(PRINT_ANY, "index", "\n\t index %u", p->index);
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 205efc9f1d9a..e1dbd3a79649 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -280,7 +280,7 @@ static int print_ife(struct action_util *au, FILE *f, 
struct rtattr *arg)
if (len) {
mtcindex =

rta_getattr_u16(metalist[IFE_META_TCINDEX]);
-   fprintf(f, "use tcindex %d ", mtcindex);
+   fprintf(f, "use tcindex %u ", mtcindex);
} else
fprintf(f, "allow tcindex ");
}
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 26549eeea899..151dfe1a230a 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -817,7 +817,7 @@ int print_pedit(struct action_util *au, FILE *f, struct 
rtattr *arg)
(unsigned int)ntohl(key->mask));
}
} else {
-   fprintf(f, "\npedit %x keys %d is not LEGIT", sel->index,
+   fprintf(f, "\npedit %x keys %u is not LEGIT", sel->index,
sel->nkeys);
}
 
diff --git a/tc/m_sample.c b/tc/m_sample.c
index 01763cb4c356..d42a6a327965 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -155,17 +155,17 @@ static int print_sample(struct action_util *au, FILE *f, 
struct rtattr *arg)
}
p = RTA_DATA(tb[TCA_SAMPLE_PARMS]);
 
-   fprintf(f, "sample rate 1/%d group %d",
+   fprintf(f, "sample rate 1/%u group %u",
rta_getattr_u32(tb[TCA_SAMPLE_RATE]),
rta_getattr_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]));
 
if (tb[TCA_SAMPLE_TRUNC_SIZE])
-   fprintf(f, " trunc_size %d",
+   fprintf(f, " trunc_size %u",
rta_getattr_u32(tb[TCA_SAMPLE_TRUNC_SIZE]));
 
print_action_control(f, " ", p->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", p->index, p->refcnt,
+   fprintf(f, "\n\tindex %u ref %d bind %d", p->index, p->refcnt,
p->bindcnt);
 
if (show_stats) {
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index 1cdd03560c35..dd8f8e8c635b 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -292,7 +292,7 @@ static int print_tunnel_key(struct action_util *au, FILE 
*f, struct rtattr *arg)
}
print_action_control(f, " ", parm->action, "");
 
-   fprintf(f, "\n\tindex %d ref %d bind %d", parm->index, parm->refcnt,
+   fprintf(f, "\n\tindex %u ref %d bind %d", parm->index, parm->refcnt,
parm->bindcnt);
 
if (show_stats) {
-- 
2.7.4

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

[Roman Mashak]

Liran Alon  writes:


[...]

>> Overall I think it might be nice to not need scrubbing skb in such
>> cases,
>> although my concern would be that this has potential to break
>> existing
>> setups when they would expect mark being zero on other veth peer in
>> any
>> case since it's the behavior for a long time already. The safer
>> option
>> would be to have some sort of explicit opt-in e.g. on link creation to
>> let
>> the skb->mark pass through unscrubbed. This would definitely be a
>> useful
>> option e.g. when mark is set in the netns facing veth via
>> clsact/egress
>> on xmit and when the container is unprivileged anyway.
>> 
>> Thanks,
>> Daniel
>
> I see your point in regards to backwards comparability.
> However, not scrubbing skb when it cross netns via some kernel functions 
> compared to
> others is basically a bug which could easily break with a little bit of more 
> refactoring.
> Therefore, it seems a bit weird to me to from now on, we will force
> every user on link creation to consider that once there was a bug leading
> to this weird behavior on specific netdevs.
> Thus, I suggest to maybe control this via a global /proc/sys/net file instead.

One valid use case could be preserving a source namespace nsid in
skb->mark when a packet crosses netns.

Re: [PATCH net] net/sched: fix NULL dereference in the error path of tcf_vlan_init()

[Roman Mashak]

Davide Caratti  writes:

> when the following command
>
>  # tc actions replace action vlan pop index 100
>
> is run for the first time, and tcf_vlan_init() fails allocating struct
> tcf_vlan_params, tcf_vlan_cleanup() calls kfree_rcu(NULL, ...). This causes
> the following error:
>

[...]

> fix this in tcf_vlan_cleanup(), ensuring that kfree_rcu(p, ...) is called
> only when p is not NULL.
>
> Fixes: 4c5b9d9642c8 ("act_vlan: VLAN action rewrite to use RCU lock/unlock 
> and update")
> Signed-off-by: Davide Caratti 
> ---
>  net/sched/act_vlan.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index e1a1b3f3983a..c2914e9a4a6f 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -225,7 +225,8 @@ static void tcf_vlan_cleanup(struct tc_action *a)
>   struct tcf_vlan_params *p;
>  
>   p = rcu_dereference_protected(v->vlan_p, 1);
> - kfree_rcu(p, rcu);
> + if (p)
> + kfree_rcu(p, rcu);
>  }
>  
>  static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,

Good catch. I think you can propagate the fix on the other actions
->cleanup(), where private parameters structure may not be present at
cleanup time, e.g. csum, ife.

[PATCH iproute2 1/1] tc: use get_u32() in psample action to match types

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_sample.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tc/m_sample.c b/tc/m_sample.c
index ff5ee6bd1ef6..dff986f5 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -65,7 +65,7 @@ static int parse_sample(struct action_util *a, int *argc_p, 
char ***argv_p,
while (argc > 0) {
if (matches(*argv, "rate") == 0) {
NEXT_ARG();
-   if (get_unsigned(, *argv, 10) != 0) {
+   if (get_u32(, *argv, 10) != 0) {
fprintf(stderr, "Illegal rate %s\n", *argv);
usage();
return -1;
@@ -73,7 +73,7 @@ static int parse_sample(struct action_util *a, int *argc_p, 
char ***argv_p,
rate_set = true;
} else if (matches(*argv, "group") == 0) {
NEXT_ARG();
-   if (get_unsigned(, *argv, 10) != 0) {
+   if (get_u32(, *argv, 10) != 0) {
fprintf(stderr, "Illegal group num %s\n",
*argv);
usage();
@@ -82,7 +82,7 @@ static int parse_sample(struct action_util *a, int *argc_p, 
char ***argv_p,
group_set = true;
} else if (matches(*argv, "trunc") == 0) {
NEXT_ARG();
-   if (get_unsigned(, *argv, 10) != 0) {
+   if (get_u32(, *argv, 10) != 0) {
fprintf(stderr, "Illegal truncation size %s\n",
*argv);
usage();
-- 
2.7.4

[PATCH v2 net 1/1] net sched actions: return explicit error when tunnel_key mode is not specified

[Roman Mashak]

If set/unset mode of the tunnel_key action is not provided, ->init() still
returns 0, and the caller proceeds with bogus 'struct tc_action *' object,
this results in crash:

% tc actions add action tunnel_key src_ip 1.1.1.1 dst_ip 2.2.2.1 id 7 index 1

[   35.805515] general protection fault:  [#1] SMP PTI
[   35.806161] Modules linked in: act_tunnel_key kvm_intel kvm irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64
crypto_simd glue_helper cryptd serio_raw
[   35.808233] CPU: 1 PID: 428 Comm: tc Not tainted 4.16.0-rc4+ #286
[   35.808929] RIP: 0010:tcf_action_init+0x90/0x190
[   35.809457] RSP: 0018:b8edc068b9a0 EFLAGS: 00010206
[   35.810053] RAX: 1320c00a0003 RBX: 0001 RCX: 
[   35.810866] RDX: 0070 RSI: 7965 RDI: b8edc068b910
[   35.811660] RBP: b8edc068b9d0 R08:  R09: b8edc068b808
[   35.812463] R10: c02bf040 R11: 0040 R12: b8edc068bb38
[   35.813235] R13:  R14:  R15: b8edc068b910
[   35.814006] FS:  7f3d0d8556c0() GS:91d1dbc4()
knlGS:
[   35.814881] CS:  0010 DS:  ES:  CR0: 80050033
[   35.815540] CR2: 0043f720 CR3: 19248001 CR4: 001606a0
[   35.816457] Call Trace:
[   35.817158]  tc_ctl_action+0x11a/0x220
[   35.817795]  rtnetlink_rcv_msg+0x23d/0x2e0
[   35.818457]  ? __slab_alloc+0x1c/0x30
[   35.819079]  ? __kmalloc_node_track_caller+0xb1/0x2b0
[   35.819544]  ? rtnl_calcit.isra.30+0xe0/0xe0
[   35.820231]  netlink_rcv_skb+0xce/0x100
[   35.820744]  netlink_unicast+0x164/0x220
[   35.821500]  netlink_sendmsg+0x293/0x370
[   35.822040]  sock_sendmsg+0x30/0x40
[   35.822508]  ___sys_sendmsg+0x2c5/0x2e0
[   35.823149]  ? pagecache_get_page+0x27/0x220
[   35.823714]  ? filemap_fault+0xa2/0x640
[   35.824423]  ? page_add_file_rmap+0x108/0x200
[   35.825065]  ? alloc_set_pte+0x2aa/0x530
[   35.825585]  ? finish_fault+0x4e/0x70
[   35.826140]  ? __handle_mm_fault+0xbc1/0x10d0
[   35.826723]  ? __sys_sendmsg+0x41/0x70
[   35.827230]  __sys_sendmsg+0x41/0x70
[   35.827710]  do_syscall_64+0x68/0x120
[   35.828195]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[   35.828859] RIP: 0033:0x7f3d0ca4da67
[   35.829331] RSP: 002b:7ffc9f284338 EFLAGS: 0246 ORIG_RAX:
002e
[   35.830304] RAX: ffda RBX: 7ffc9f284460 RCX: 7f3d0ca4da67
[   35.831247] RDX:  RSI: 7ffc9f2843b0 RDI: 0003
[   35.832167] RBP: 5aa6a7a9 R08: 0001 R09: 
[   35.833075] R10: 05f1 R11: 0246 R12: 
[   35.833997] R13: 7ffc9f2884c0 R14: 0001 R15: 00674640
[   35.834923] Code: 24 30 bb 01 00 00 00 45 31 f6 eb 5e 8b 50 08 83 c2 07 83 e2
fc 83 c2 70 49 8b 07 48 8b 40 70 48 85 c0 74 10 48 89 14 24 4c 89 ff  d0 48
8b 14 24 48 01 c2 49 01 d6 45 85 ed 74 05 41 83 47 2c 
[   35.837442] RIP: tcf_action_init+0x90/0x190 RSP: b8edc068b9a0
[   35.838291] ---[ end trace a095c06ee4b97a26 ]---

v2:
Submit for net tree

Fixes: d0f6dd8a914f ("net/sched: Introduce act_tunnel_key")
Signed-off-by: Roman Mashak <m...@mojatatu.com>
Acked-by: Cong Wang <xiyou.wangc...@gmail.com>
---
 net/sched/act_tunnel_key.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 0e23aac09ad6..fea772e66e62 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -153,6 +153,7 @@ static int tunnel_key_init(struct net *net, struct nlattr 
*nla,
metadata->u.tun_info.mode |= IP_TUNNEL_INFO_TX;
break;
default:
+   ret = -EINVAL;
goto err_out;
}
 
-- 
2.7.4

[PATCH iproute2 1/1] tc: print actual action for sample action

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/m_sample.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tc/m_sample.c b/tc/m_sample.c
index ff5ee6bd1ef6..d88846c63be3 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -163,6 +163,8 @@ static int print_sample(struct action_util *au, FILE *f, 
struct rtattr *arg)
fprintf(f, " trunc_size %d",
rta_getattr_u32(tb[TCA_SAMPLE_TRUNC_SIZE]));
 
+   print_action_control(f, " ", p->action, "");
+
fprintf(f, "\n\tindex %d ref %d bind %d", p->index, p->refcnt,
p->bindcnt);
 
-- 
2.7.4

[PATCH net-next 1/1] net sched actions: return explicit error when tunnel_key mode is not specified

[Roman Mashak]

If set/unset mode of the tunnel_key action is not provided, ->init() still
returns 0, and the caller proceeds with bogus 'struct tc_action *' object,
this results in crash:

% tc actions add action tunnel_key src_ip 1.1.1.1 dst_ip 2.2.2.1 id 7 index 1

[   35.805515] general protection fault:  [#1] SMP PTI
[   35.806161] Modules linked in: act_tunnel_key kvm_intel kvm irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64
crypto_simd glue_helper cryptd serio_raw
[   35.808233] CPU: 1 PID: 428 Comm: tc Not tainted 4.16.0-rc4+ #286
[   35.808929] RIP: 0010:tcf_action_init+0x90/0x190
[   35.809457] RSP: 0018:b8edc068b9a0 EFLAGS: 00010206
[   35.810053] RAX: 1320c00a0003 RBX: 0001 RCX: 
[   35.810866] RDX: 0070 RSI: 7965 RDI: b8edc068b910
[   35.811660] RBP: b8edc068b9d0 R08:  R09: b8edc068b808
[   35.812463] R10: c02bf040 R11: 0040 R12: b8edc068bb38
[   35.813235] R13:  R14:  R15: b8edc068b910
[   35.814006] FS:  7f3d0d8556c0() GS:91d1dbc4()
knlGS:
[   35.814881] CS:  0010 DS:  ES:  CR0: 80050033
[   35.815540] CR2: 0043f720 CR3: 19248001 CR4: 001606a0
[   35.816457] Call Trace:
[   35.817158]  tc_ctl_action+0x11a/0x220
[   35.817795]  rtnetlink_rcv_msg+0x23d/0x2e0
[   35.818457]  ? __slab_alloc+0x1c/0x30
[   35.819079]  ? __kmalloc_node_track_caller+0xb1/0x2b0
[   35.819544]  ? rtnl_calcit.isra.30+0xe0/0xe0
[   35.820231]  netlink_rcv_skb+0xce/0x100
[   35.820744]  netlink_unicast+0x164/0x220
[   35.821500]  netlink_sendmsg+0x293/0x370
[   35.822040]  sock_sendmsg+0x30/0x40
[   35.822508]  ___sys_sendmsg+0x2c5/0x2e0
[   35.823149]  ? pagecache_get_page+0x27/0x220
[   35.823714]  ? filemap_fault+0xa2/0x640
[   35.824423]  ? page_add_file_rmap+0x108/0x200
[   35.825065]  ? alloc_set_pte+0x2aa/0x530
[   35.825585]  ? finish_fault+0x4e/0x70
[   35.826140]  ? __handle_mm_fault+0xbc1/0x10d0
[   35.826723]  ? __sys_sendmsg+0x41/0x70
[   35.827230]  __sys_sendmsg+0x41/0x70
[   35.827710]  do_syscall_64+0x68/0x120
[   35.828195]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[   35.828859] RIP: 0033:0x7f3d0ca4da67
[   35.829331] RSP: 002b:7ffc9f284338 EFLAGS: 0246 ORIG_RAX:
002e
[   35.830304] RAX: ffda RBX: 7ffc9f284460 RCX: 7f3d0ca4da67
[   35.831247] RDX:  RSI: 7ffc9f2843b0 RDI: 0003
[   35.832167] RBP: 5aa6a7a9 R08: 0001 R09: 
[   35.833075] R10: 05f1 R11: 0246 R12: 
[   35.833997] R13: 7ffc9f2884c0 R14: 0001 R15: 00674640
[   35.834923] Code: 24 30 bb 01 00 00 00 45 31 f6 eb 5e 8b 50 08 83 c2 07 83 e2
fc 83 c2 70 49 8b 07 48 8b 40 70 48 85 c0 74 10 48 89 14 24 4c 89 ff  d0 48
8b 14 24 48 01 c2 49 01 d6 45 85 ed 74 05 41 83 47 2c 
[   35.837442] RIP: tcf_action_init+0x90/0x190 RSP: b8edc068b9a0
[   35.838291] ---[ end trace a095c06ee4b97a26 ]---

Fixes: d0f6dd8a914f ("net/sched: Introduce act_tunnel_key")
Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_tunnel_key.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 9169b7e78ada..0c0fbbfbc63b 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -153,6 +153,7 @@ static int tunnel_key_init(struct net *net, struct nlattr 
*nla,
metadata->u.tun_info.mode |= IP_TUNNEL_INFO_TX;
break;
default:
+   ret = -EINVAL;
goto err_out;
}
 
-- 
2.7.4

[RESEND PATCH net-next 1/1] tc-testing: updated gact tests with batch test cases

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/gact.json  | 73 +-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
index e2187b6e0b7a..ae96d0350d7e 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
@@ -465,5 +465,76 @@
 "teardown": [
 "$TC actions flush action gact"
 ]
+},
+{
+"id": "1021",
+"name": "Add batch of 32 gact pass actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action pass index $i 
\"; args=\"$args$cmd\"; done && $TC actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "da7a",
+"name": "Add batch of 32 gact continue actions with cookie",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action continue index 
$i cookie aabbccddeeff112233445566778800a1 \"; args=\"$args$cmd\"; done && $TC 
actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "8aa3",
+"name": "Delete batch of 32 gact continue actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+],
+"for i in `seq 1 32`; do cmd=\"action continue index $i \"; 
args=\"$args$cmd\"; done && $TC actions add $args"
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action gact index $i 
\"; args=\"$args$cmd\"; done && $TC actions del $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "0",
+"teardown": []
 }
-]
+]
\ No newline at end of file
-- 
2.7.4

[RESEND PATCH net-next 1/1] tc-testing: add TC vlan action tests

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/vlan.json  | 410 +
 1 file changed, 410 insertions(+)
 create mode 100644 
tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
new file mode 100644
index ..4510ddfa6e54
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
@@ -0,0 +1,410 @@
+[
+{
+"id": "6f5a",
+"name": "Add vlan pop action",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop index 8",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*pop.*index 8 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "ee6f",
+"name": "Add vlan pop action with large index",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop index 4294967295",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*pop.*index 4294967295 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "b6b9",
+"name": "Add vlan pop action with jump opcode",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop jump 10 index 8",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*jump 10.*index 8 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "87c3",
+"name": "Add vlan pop action with trap opcode",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop trap index 8",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*pop trap.*index 8 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "2b91",
+"name": "Add vlan invalid action",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan bad_mode",
+"expExitCode": "255",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*bad_mode",
+"matchCount": "0",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+   

Re: [PATCH net-next 1/1] tc-testing: updated gact tests with batch test cases

[Roman Mashak]

David Miller <da...@davemloft.net> writes:

> From: Roman Mashak <m...@mojatatu.com>
> Date: Mon, 12 Mar 2018 12:39:57 -0400
>
>> David Miller <da...@davemloft.net> writes:
>> 
>>> From: Roman Mashak <m...@mojatatu.com>
>>> Date: Fri,  9 Mar 2018 17:16:12 -0500
>>>
>>>> Add test cases to exercise code paths responsible for adding or deleting
>>>> batch of TC actions.
>>>> 
>>>> Signed-off-by: Roman Mashak <m...@mojatatu.com>
>>>
>>> You submitted this patch twice.
>>>
>>> You didn't indicate if this is a new version of the patch or something
>>> like this.
>>>
>> 
>> Sorry for that glitch, I've just resent the patch.
>> 
>>> Please resubmit your tc-testing changes, all of them, with this
>>> sorted out.
>
> I said "all of your tc-testing changes" not just this one.

On March 9th I submitted two tc-testing patches:

 - "add TC vlan action tests"
 - "updated gact tests with batch test cases" (which I resent)

Do you want to have those submitted as series?

[RESEND PATCH net-next 1/1] tc-testing: updated gact tests with batch test cases

[Roman Mashak]

Add test cases to exercise code paths responsible for adding or deleting
batch of TC actions.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/gact.json  | 73 +-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
index e2187b6e0b7a..ae96d0350d7e 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
@@ -465,5 +465,76 @@
 "teardown": [
 "$TC actions flush action gact"
 ]
+},
+{
+"id": "1021",
+"name": "Add batch of 32 gact pass actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action pass index $i 
\"; args=\"$args$cmd\"; done && $TC actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "da7a",
+"name": "Add batch of 32 gact continue actions with cookie",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action continue index 
$i cookie aabbccddeeff112233445566778800a1 \"; args=\"$args$cmd\"; done && $TC 
actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "8aa3",
+"name": "Delete batch of 32 gact continue actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+],
+"for i in `seq 1 32`; do cmd=\"action continue index $i \"; 
args=\"$args$cmd\"; done && $TC actions add $args"
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action gact index $i 
\"; args=\"$args$cmd\"; done && $TC actions del $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "0",
+"teardown": []
 }
-]
+]
\ No newline at end of file
-- 
2.7.4

Re: [PATCH net-next 1/1] tc-testing: updated gact tests with batch test cases

[Roman Mashak]

David Miller <da...@davemloft.net> writes:

> From: Roman Mashak <m...@mojatatu.com>
> Date: Fri,  9 Mar 2018 17:16:12 -0500
>
>> Add test cases to exercise code paths responsible for adding or deleting
>> batch of TC actions.
>> 
>> Signed-off-by: Roman Mashak <m...@mojatatu.com>
>
> You submitted this patch twice.
>
> You didn't indicate if this is a new version of the patch or something
> like this.
>

Sorry for that glitch, I've just resent the patch.

> Please resubmit your tc-testing changes, all of them, with this
> sorted out.

Re: [PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL

[Roman Mashak]

Stephen Hemminger <step...@networkplumber.org> writes:

> On Sat, 10 Mar 2018 08:31:26 -0500
> Roman Mashak <m...@mojatatu.com> wrote:
>
>> Signed-off-by: Roman Mashak <m...@mojatatu.com>
>> ---
>>  tc/f_flower.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tc/f_flower.c b/tc/f_flower.c
>> index 5a4ec832bc19..5cf19139dcae 100644
>> --- a/tc/f_flower.c
>> +++ b/tc/f_flower.c
>> @@ -1131,7 +1131,7 @@ static void flower_print_ip_attr(char *name, struct 
>> rtattr *key_attr,
>>  if (mask_attr)
>>  sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
>>  
>> -sprintf(namefrm, "\n  %s %%x", name);
>> +sprintf(namefrm, "\n  %s %%s", name);
>>  print_string(PRINT_ANY, name, namefrm, out);
>>  }
>>  
>
> I don't see why code has to build a format string here, and not do what other 
> code does
> and do multiple print's. Plus for JSON it is better to not create aggregated 
> types.
>
> Also sprintf() should not be used only snprintf.
>
> And the flower code seems to have forgotten the seldom used oneline mode.
>
> Maybe something like:
>
> -static void flower_print_ip_attr(char *name, struct rtattr *key_attr,
> +static void flower_print_ip_attr(const char *name, struct rtattr *key_attr,
>  struct rtattr *mask_attr)
>  {
> -   SPRINT_BUF(namefrm);
> -   SPRINT_BUF(out);
> -   size_t done;
> +   SPRINT_BUF(mask_name);
>  
> if (!key_attr)
> return;
>  
> -   done = sprintf(out, "%x", rta_getattr_u8(key_attr));
> -   if (mask_attr)
> -   sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
> +   print_string(PRINT_FP, NULL, "%s  ", _SL_);
> +   print_string(PRINT_FP, NULL, "%s ", name);
> +   print_0xhex(PRINT_ANY, name, "%x", rta_getattr_u8(key_attr));
>  
> -   sprintf(namefrm, "\n  %s %%x", name);
> -   print_string(PRINT_ANY, name, namefrm, out);
> +   if (mask_attr) {
> +   snprintf(mask_name, sizeof(mask_name),
> +"%s_mask", name);
> +   print_0xhex(PRINT_ANY, mask_name, "/%x",
> +rta_getattr_u8(mask_attr));
> +   }
>  }
>  
>  static void flower_print_matching_flags(char *name,

Hi Stephen, thanks for suggestion. Would you submit your patch, or
should I prepare v2? Also, 'oneline' mode isn't supported in tc at the
moment, so needs to be added for consistency with ip or bridge.

[PATCH iproute2 1/1] tc: fix output when printing flower's TOS or TTL

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 tc/f_flower.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 5a4ec832bc19..5cf19139dcae 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1131,7 +1131,7 @@ static void flower_print_ip_attr(char *name, struct 
rtattr *key_attr,
if (mask_attr)
sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
 
-   sprintf(namefrm, "\n  %s %%x", name);
+   sprintf(namefrm, "\n  %s %%s", name);
print_string(PRINT_ANY, name, namefrm, out);
 }
 
-- 
2.7.4

[PATCH net-next 1/1] tc-testing: updated gact tests with batch test cases

[Roman Mashak]

Add test cases to exercise code paths responsible for adding or deleting
batch of TC actions.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/gact.json  | 73 +-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
index e2187b6e0b7a..ae96d0350d7e 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
@@ -465,5 +465,76 @@
 "teardown": [
 "$TC actions flush action gact"
 ]
+},
+{
+"id": "1021",
+"name": "Add batch of 32 gact pass actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action pass index $i 
\"; args=\"$args$cmd\"; done && $TC actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "da7a",
+"name": "Add batch of 32 gact continue actions with cookie",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action continue index 
$i cookie aabbccddeeff112233445566778800a1 \"; args=\"$args$cmd\"; done && $TC 
actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "8aa3",
+"name": "Delete batch of 32 gact continue actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+],
+"for i in `seq 1 32`; do cmd=\"action continue index $i \"; 
args=\"$args$cmd\"; done && $TC actions add $args"
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action gact index $i 
\"; args=\"$args$cmd\"; done && $TC actions del $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "0",
+"teardown": []
 }
-]
+]
\ No newline at end of file
-- 
2.7.4

[PATCH net-next 1/1] tc-testing: add TC vlan action tests

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
Tested-by: Brenda J. Butler <b...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/vlan.json  | 410 +
 1 file changed, 410 insertions(+)
 create mode 100644 
tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
new file mode 100644
index ..4510ddfa6e54
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/vlan.json
@@ -0,0 +1,410 @@
+[
+{
+"id": "6f5a",
+"name": "Add vlan pop action",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop index 8",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*pop.*index 8 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "ee6f",
+"name": "Add vlan pop action with large index",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop index 4294967295",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*pop.*index 4294967295 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "b6b9",
+"name": "Add vlan pop action with jump opcode",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop jump 10 index 8",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*jump 10.*index 8 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "87c3",
+"name": "Add vlan pop action with trap opcode",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan pop trap index 8",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*pop trap.*index 8 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action vlan"
+]
+},
+{
+"id": "2b91",
+"name": "Add vlan invalid action",
+"category": [
+"actions",
+"vlan"
+],
+"setup": [
+[
+"$TC actions flush action vlan",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action vlan bad_mode",
+"expExitCode": "255",
+"verifyCmd": "$TC actions list action vlan",
+"matchPattern": "action order [0-9]+: vlan.*bad_mode",
+"matchCount": "0",
+"teardown": [
+"$TC

[PATCH net-next 1/1] tc-testing: updated gact tests with batch test cases

[Roman Mashak]

Add test cases to exercise code paths responsible for adding or deleting
batch of TC actions.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/gact.json  | 73 +-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
index e2187b6e0b7a..ae96d0350d7e 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
@@ -465,5 +465,76 @@
 "teardown": [
 "$TC actions flush action gact"
 ]
+},
+{
+"id": "1021",
+"name": "Add batch of 32 gact pass actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action pass index $i 
\"; args=\"$args$cmd\"; done && $TC actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "da7a",
+"name": "Add batch of 32 gact continue actions with cookie",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action continue index 
$i cookie aabbccddeeff112233445566778800a1 \"; args=\"$args$cmd\"; done && $TC 
actions add $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "32",
+"teardown": [
+"$TC actions flush action gact"
+]
+},
+{
+"id": "8aa3",
+"name": "Delete batch of 32 gact continue actions",
+"category": [
+"actions",
+"gact"
+],
+"setup": [
+[
+"$TC actions flush action gact",
+0,
+1,
+255
+],
+"for i in `seq 1 32`; do cmd=\"action continue index $i \"; 
args=\"$args$cmd\"; done && $TC actions add $args"
+],
+"cmdUnderTest": "for i in `seq 1 32`; do cmd=\"action gact index $i 
\"; args=\"$args$cmd\"; done && $TC actions del $args",
+"expExitCode": "0",
+"verifyCmd": "$TC actions list action gact",
+"matchPattern": "^[ \t]+index [0-9]+ ref",
+"matchCount": "0",
+"teardown": []
 }
-]
+]
\ No newline at end of file
-- 
2.7.4

[PATCH v3 net-next 3/4] net sched actions: calculate add/delete event message size

[Roman Mashak]

Introduce routines to calculate size of the shared tc netlink attributes
and the full message size including netlink header and tc service header.

Update add/delete action logic to have the size for event messages,
the size is passed to tcf_add_notify() and tcf_del_notify() where the
notification message is being allocated and constructed.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_api.c | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 3de0e0610200..57cf37145282 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -109,6 +109,42 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool 
strict)
 }
 EXPORT_SYMBOL(__tcf_idr_release);
 
+static size_t tcf_action_shared_attrs_size(const struct tc_action *act)
+{
+   u32 cookie_len = 0;
+
+   if (act->act_cookie)
+   cookie_len = nla_total_size(act->act_cookie->len);
+
+   return  nla_total_size(0) /* action number nested */
+   + nla_total_size(IFNAMSIZ) /* TCA_ACT_KIND */
+   + cookie_len /* TCA_ACT_COOKIE */
+   + nla_total_size(0) /* TCA_ACT_STATS nested */
+   /* TCA_STATS_BASIC */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_basic))
+   /* TCA_STATS_QUEUE */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_queue))
+   + nla_total_size(0) /* TCA_OPTIONS nested */
+   + nla_total_size(sizeof(struct tcf_t)); /* TCA_GACT_TM */
+}
+
+static size_t tcf_action_full_attrs_size(size_t sz)
+{
+   return NLMSG_HDRLEN /* struct nlmsghdr */
+   + sizeof(struct tcamsg)
+   + nla_total_size(0) /* TCA_ACT_TAB nested */
+   + sz;
+}
+
+static size_t tcf_action_fill_size(const struct tc_action *act)
+{
+   size_t sz = tcf_action_shared_attrs_size(act);
+
+   if (act->ops->get_fill_size)
+   return act->ops->get_fill_size(act) + sz;
+   return sz;
+}
+
 static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
   struct netlink_callback *cb)
 {
@@ -746,6 +782,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, 
struct nlattr *nla,
 {
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+   size_t sz = 0;
int err;
int i;
 
@@ -761,11 +798,14 @@ int tcf_action_init(struct net *net, struct tcf_proto 
*tp, struct nlattr *nla,
goto err;
}
act->order = i;
+   sz += tcf_action_fill_size(act);
if (ovr)
act->tcfa_refcnt++;
list_add_tail(>list, actions);
}
 
+   *attr_size = tcf_action_full_attrs_size(sz);
+
/* Remove the temp refcnt which was necessary to protect against
 * destroying an existing action which was being replaced
 */
@@ -1056,9 +1096,12 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
goto err;
}
act->order = i;
+   attr_size += tcf_action_fill_size(act);
list_add_tail(>list, );
}
 
+   attr_size = tcf_action_full_attrs_size(attr_size);
+
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, , event, extack);
else { /* delete */
-- 
2.7.4

[PATCH v3 net-next 4/4] net sched actions: implement get_fill_size routine in act_gact

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_gact.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 74563254e676..88fbb8403565 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -217,6 +217,19 @@ static int tcf_gact_search(struct net *net, struct 
tc_action **a, u32 index,
return tcf_idr_search(tn, a, index);
 }
 
+static size_t tcf_gact_get_fill_size(const struct tc_action *act)
+{
+   size_t sz = nla_total_size(sizeof(struct tc_gact)); /* TCA_GACT_PARMS */
+
+#ifdef CONFIG_GACT_PROB
+   if (to_gact(act)->tcfg_ptype)
+   /* TCA_GACT_PROB */
+   sz += nla_total_size(sizeof(struct tc_gact_p));
+#endif
+
+   return sz;
+}
+
 static struct tc_action_ops act_gact_ops = {
.kind   =   "gact",
.type   =   TCA_ACT_GACT,
@@ -227,6 +240,7 @@ static struct tc_action_ops act_gact_ops = {
.init   =   tcf_gact_init,
.walk   =   tcf_gact_walker,
.lookup =   tcf_gact_search,
+   .get_fill_size  =   tcf_gact_get_fill_size,
.size   =   sizeof(struct tcf_gact),
 };
 
-- 
2.7.4

[PATCH v3 net-next 2/4] net sched actions: add new tc_action_ops callback

[Roman Mashak]

Add a new callback in tc_action_ops, it will be needed by the tc actions
to compute its size when a ADD/DELETE notification message is constructed.
This routine has to take into account optional/variable size TLVs specific
per action.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 include/net/act_api.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 88c1f99bae46..e0a9c2003b24 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -97,6 +97,7 @@ struct tc_action_ops {
const struct tc_action_ops *,
struct netlink_ext_ack *);
void(*stats_update)(struct tc_action *, u64, u32, u64);
+   size_t  (*get_fill_size)(const struct tc_action *act);
struct net_device *(*get_dev)(const struct tc_action *a);
 };
 
-- 
2.7.4

[PATCH v3 net-next 0/4] Fix event generation for actions batch Add/Delete mode

[Roman Mashak]

When adding or deleting a batch of entries, the kernel sends upto
TCA_ACT_MAX_PRIO entries in an event to user space. However it does not
consider that the action sizes may vary and require different skb sizes.

For example :

% cat tc-batch.sh
#!/bin/bash
TC="sudo /mnt/iproute2.git/tc/tc"

$TC actions flush action gact
for i in `seq 1 $1`;
do
   cmd="action pass index $i "
   args=$args$cmd
done
$TC actions add $args
%
% ./tc-batch.sh 32
Error: Failed to fill netlink attributes while adding TC action.
We have an error talking to the kernel
%

This patchset introduces new callback in tc_action_ops, which calculates
the action size, and passes size to tcf_add_notify()/tcf_del_notify(). The
patch fixes act_gact, and the rest of actions will be updated in the
follow-up patches.

v3:
   Fixed tcf_action_fill_size() to return shared attrs length when
   action ->get_fill_size() isn't implemented.
v2:
   Restructured patches to make them bisectable.

Roman Mashak (4):
  net sched actions: update Add/Delete action API with new argument
  net sched actions: add new tc_action_ops callback
  net sched actions: calculate add/delete event message size
  net sched actions: implement get_fill_size routine in act_gact

 include/net/act_api.h |  4 +++-
 net/sched/act_api.c   | 63 ---
 net/sched/act_gact.c  | 14 
 net/sched/cls_api.c   |  3 ++-
 4 files changed, 74 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH v3 net-next 1/4] net sched actions: update Add/Delete action API with new argument

[Roman Mashak]

Introduce a new function argument to carry total attributes size for
correct allocation of skb in event messages.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 include/net/act_api.h |  3 ++-
 net/sched/act_api.c   | 21 +
 net/sched/cls_api.c   |  3 ++-
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 9c2f22695025..88c1f99bae46 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -166,7 +166,8 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action 
**actions,
int nr_actions, struct tcf_result *res);
 int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
-   struct list_head *actions, struct netlink_ext_ack *extack);
+   struct list_head *actions, size_t *attr_size,
+   struct netlink_ext_ack *extack);
 struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
char *name, int ovr, int bind,
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a54fa7b8c217..3de0e0610200 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -741,7 +741,8 @@ static void cleanup_a(struct list_head *actions, int ovr)
 
 int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
-   struct list_head *actions, struct netlink_ext_ack *extack)
+   struct list_head *actions, size_t *attr_size,
+   struct netlink_ext_ack *extack)
 {
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
@@ -994,12 +995,13 @@ static int tca_action_flush(struct net *net, struct 
nlattr *nla,
 
 static int
 tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, struct netlink_ext_ack *extack)
+  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
 {
int ret;
struct sk_buff *skb;
 
-   skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
+   GFP_KERNEL);
if (!skb)
return -ENOBUFS;
 
@@ -1032,6 +1034,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct 
nlmsghdr *n,
int i, ret;
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+   size_t attr_size = 0;
LIST_HEAD(actions);
 
ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
@@ -1059,7 +1062,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct 
nlmsghdr *n,
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, , event, extack);
else { /* delete */
-   ret = tcf_del_notify(net, n, , portid, extack);
+   ret = tcf_del_notify(net, n, , portid, attr_size, 
extack);
if (ret)
goto err;
return ret;
@@ -1072,12 +1075,13 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
 
 static int
 tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, struct netlink_ext_ack *extack)
+  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
 {
struct sk_buff *skb;
int err = 0;
 
-   skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
+   GFP_KERNEL);
if (!skb)
return -ENOBUFS;
 
@@ -1099,15 +1103,16 @@ static int tcf_action_add(struct net *net, struct 
nlattr *nla,
  struct nlmsghdr *n, u32 portid, int ovr,
  struct netlink_ext_ack *extack)
 {
+   size_t attr_size = 0;
int ret = 0;
LIST_HEAD(actions);
 
ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, ,
- extack);
+ _size, extack);
if (ret)
return ret;
 
-   return tcf_add_notify(net, n, , portid, extack);
+   return tcf_add_notify(net, n, , portid, attr_size, extack);
 }
 
 static u32 tcaa_root_flags_allowed = TCA_FLAG_LARGE_DUMP_ON;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 19f9f421d5b7..ec5fe8ec0c3e 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1433,6 +1433,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto 
*tp, struct nlattr **tb,
 #ifdef CONFIG_NET_CLS_ACT
{
struct tc_action *act;
+   size_t attr_size = 0;
 
if (exts->police && tb[exts->police]) {
 

[PATCH net-next 1/1] tc-testing: add csum tests

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/csum.json  | 410 +
 1 file changed, 410 insertions(+)
 create mode 100644 
tools/testing/selftests/tc-testing/tc-tests/actions/csum.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/csum.json 
b/tools/testing/selftests/tc-testing/tc-tests/actions/csum.json
new file mode 100644
index ..93cf8fea8ae7
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/csum.json
@@ -0,0 +1,410 @@
+[
+{
+"id": "6d84",
+"name": "Add csum iph action",
+"category": [
+"actions",
+"csum"
+],
+"setup": [
+[
+"$TC actions flush action csum",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action csum iph index 800",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action csum index 800",
+"matchPattern": "action order [0-9]*: csum \\(iph\\) action 
pass.*index 800 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action csum"
+]
+},
+{
+"id": "1862",
+"name": "Add csum ip4h action",
+"category": [
+"actions",
+"csum"
+],
+"setup": [
+[
+"$TC actions flush action csum",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action csum ip4h index 7",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action csum index 7",
+"matchPattern": "action order [0-9]*: csum \\(iph\\) action 
pass.*index 7 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action csum"
+]
+},
+{
+"id": "15c6",
+"name": "Add csum ipv4h action",
+"category": [
+"actions",
+"csum"
+],
+"setup": [
+[
+"$TC actions flush action csum",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action csum ipv4h index 1122",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action csum index 1122",
+"matchPattern": "action order [0-9]*: csum \\(iph\\) action 
pass.*index 1122 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action csum"
+]
+},
+{
+"id": "bf47",
+"name": "Add csum icmp action",
+"category": [
+"actions",
+"csum"
+],
+"setup": [
+[
+"$TC actions flush action csum",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action csum icmp index 1",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action csum index 1",
+"matchPattern": "action order [0-9]*: csum \\(icmp\\) action 
pass.*index 1 ref",
+"matchCount": "1",
+"teardown": [
+"$TC actions flush action csum"
+]
+},
+{
+"id": "cc1d",
+"name": "Add csum igmp action",
+"category": [
+"actions",
+"csum"
+],
+"setup": [
+[
+"$TC actions flush action csum",
+0,
+1,
+255
+]
+],
+"cmdUnderTest": "$TC actions add action csum igmp index 999",
+"expExitCode": "0",
+"verifyCmd": "$TC actions get action csum index 999",
+"matchPattern": "action order [0-9]*: csum \\(igmp\\) action 
pass.*index 999 ref",
+"matchCount": "1",
+"teardown": [

Re: [PATCH v2 net-next 3/4] net sched actions: calculate add/delete event message size

[Roman Mashak]

David Miller <da...@davemloft.net> writes:

> From: Roman Mashak <m...@mojatatu.com>
> Date: Tue,  6 Mar 2018 16:55:23 -0500
>
>> +static size_t tcf_action_fill_size(const struct tc_action *act)
>> +{
>> +if (act->ops->get_fill_size)
>> +return act->ops->get_fill_size(act) +
>> +tcf_action_shared_attrs_size(act);
>> +return 0;
>> +}
>
> I don't understand this.
>
> The shared attrs should be considered regardless of whether an action
> type specific ->get_fill_size() is implemented.
>
> But instead, you return zero in that case.

Actually when action  specific ->get_fill_size() is not
implemented, I want to fall back to the current behaviour, where we
always allocate NLMSG_GOODSIZE bytes for skb. So in this case size
passed to tcf_add_notify is estimated in tcf_action_full_attrs_size()
and smaller then NLMSG_GOODSIZE (which is page size).

But may be you're right, API should return explicit size of the message,
we can't expect PAGE_SIZE to be large enough.

I will then change this in v3.

[PATCH iproute2 1/1] tc: updated tc-bpf man page

[Roman Mashak]

Added description of direct-action parameter.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 man/man8/tc-bpf.8 | 9 +
 1 file changed, 9 insertions(+)

diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
index 2e9812ede028..d311f295615b 100644
--- a/man/man8/tc-bpf.8
+++ b/man/man8/tc-bpf.8
@@ -14,6 +14,10 @@ CLS_NAME ] [
 UDS_FILE ] [
 .B verbose
 ] [
+.B direct-action
+|
+.B da
+] [
 .B skip_hw
 |
 .B skip_sw
@@ -141,6 +145,11 @@ if set, it will dump the eBPF verifier output, even if 
loading the eBPF
 program was successful. By default, only on error, the verifier log is
 being emitted to the user.
 
+.SS direct-action | da
+instructs eBPF classifier to not invoke external TC actions, instead use the
+TC actions return codes (\fBTC_ACT_OK\fR, \fBTC_ACT_SHOT\fR etc.) for
+classifiers.
+
 .SS skip_hw | skip_sw
 hardware offload control flags. By default TC will try to offload
 filters to hardware if possible.
-- 
2.7.4

[PATCH v2 net-next 3/4] net sched actions: calculate add/delete event message size

[Roman Mashak]

Introduce routines to calculate size of the shared tc netlink attributes
and the full message size including netlink header and tc service header.

Update add/delete action logic to have the size for event messages,
the size is passed to tcf_add_notify() and tcf_del_notify() where the
notification message is being allocated and constructed.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_api.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 3de0e06..8809e48 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -109,6 +109,41 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool 
strict)
 }
 EXPORT_SYMBOL(__tcf_idr_release);
 
+static size_t tcf_action_shared_attrs_size(const struct tc_action *act)
+{
+   u32 cookie_len = 0;
+
+   if (act->act_cookie)
+   cookie_len = nla_total_size(act->act_cookie->len);
+
+   return  nla_total_size(0) /* action number nested */
+   + nla_total_size(IFNAMSIZ) /* TCA_ACT_KIND */
+   + cookie_len /* TCA_ACT_COOKIE */
+   + nla_total_size(0) /* TCA_ACT_STATS nested */
+   /* TCA_STATS_BASIC */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_basic))
+   /* TCA_STATS_QUEUE */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_queue))
+   + nla_total_size(0) /* TCA_OPTIONS nested */
+   + nla_total_size(sizeof(struct tcf_t)); /* TCA_GACT_TM */
+}
+
+static size_t tcf_action_full_attrs_size(size_t sz)
+{
+   return NLMSG_HDRLEN /* struct nlmsghdr */
+   + sizeof(struct tcamsg)
+   + nla_total_size(0) /* TCA_ACT_TAB nested */
+   + sz;
+}
+
+static size_t tcf_action_fill_size(const struct tc_action *act)
+{
+   if (act->ops->get_fill_size)
+   return act->ops->get_fill_size(act) +
+   tcf_action_shared_attrs_size(act);
+   return 0;
+}
+
 static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
   struct netlink_callback *cb)
 {
@@ -746,6 +781,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, 
struct nlattr *nla,
 {
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+   size_t sz = 0;
int err;
int i;
 
@@ -761,11 +797,14 @@ int tcf_action_init(struct net *net, struct tcf_proto 
*tp, struct nlattr *nla,
goto err;
}
act->order = i;
+   sz += tcf_action_fill_size(act);
if (ovr)
act->tcfa_refcnt++;
list_add_tail(>list, actions);
}
 
+   *attr_size = tcf_action_full_attrs_size(sz);
+
/* Remove the temp refcnt which was necessary to protect against
 * destroying an existing action which was being replaced
 */
@@ -1056,9 +1095,12 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
goto err;
}
act->order = i;
+   attr_size += tcf_action_fill_size(act);
list_add_tail(>list, );
}
 
+   attr_size = tcf_action_full_attrs_size(attr_size);
+
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, , event, extack);
else { /* delete */
-- 
2.7.4

[PATCH v2 net-next 1/4] net sched actions: update Add/Delete action API with new argument

[Roman Mashak]

Introduce a new function argument to carry total attributes size for
correct allocation of skb in event messages.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 include/net/act_api.h |  3 ++-
 net/sched/act_api.c   | 21 +
 net/sched/cls_api.c   |  3 ++-
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 9c2f226..88c1f99 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -166,7 +166,8 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action 
**actions,
int nr_actions, struct tcf_result *res);
 int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
-   struct list_head *actions, struct netlink_ext_ack *extack);
+   struct list_head *actions, size_t *attr_size,
+   struct netlink_ext_ack *extack);
 struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
char *name, int ovr, int bind,
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a54fa7b..3de0e06 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -741,7 +741,8 @@ static void cleanup_a(struct list_head *actions, int ovr)
 
 int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
-   struct list_head *actions, struct netlink_ext_ack *extack)
+   struct list_head *actions, size_t *attr_size,
+   struct netlink_ext_ack *extack)
 {
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
@@ -994,12 +995,13 @@ static int tca_action_flush(struct net *net, struct 
nlattr *nla,
 
 static int
 tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, struct netlink_ext_ack *extack)
+  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
 {
int ret;
struct sk_buff *skb;
 
-   skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
+   GFP_KERNEL);
if (!skb)
return -ENOBUFS;
 
@@ -1032,6 +1034,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct 
nlmsghdr *n,
int i, ret;
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+   size_t attr_size = 0;
LIST_HEAD(actions);
 
ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
@@ -1059,7 +1062,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct 
nlmsghdr *n,
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, , event, extack);
else { /* delete */
-   ret = tcf_del_notify(net, n, , portid, extack);
+   ret = tcf_del_notify(net, n, , portid, attr_size, 
extack);
if (ret)
goto err;
return ret;
@@ -1072,12 +1075,13 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
 
 static int
 tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, struct netlink_ext_ack *extack)
+  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
 {
struct sk_buff *skb;
int err = 0;
 
-   skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
+   GFP_KERNEL);
if (!skb)
return -ENOBUFS;
 
@@ -1099,15 +1103,16 @@ static int tcf_action_add(struct net *net, struct 
nlattr *nla,
  struct nlmsghdr *n, u32 portid, int ovr,
  struct netlink_ext_ack *extack)
 {
+   size_t attr_size = 0;
int ret = 0;
LIST_HEAD(actions);
 
ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, ,
- extack);
+ _size, extack);
if (ret)
return ret;
 
-   return tcf_add_notify(net, n, , portid, extack);
+   return tcf_add_notify(net, n, , portid, attr_size, extack);
 }
 
 static u32 tcaa_root_flags_allowed = TCA_FLAG_LARGE_DUMP_ON;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 19f9f42..ec5fe8e 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1433,6 +1433,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto 
*tp, struct nlattr **tb,
 #ifdef CONFIG_NET_CLS_ACT
{
struct tc_action *act;
+   size_t attr_size = 0;
 
if (exts->police && tb[exts->police]) {
act = tcf_acti

[PATCH v2 net-next 4/4] net sched actions: implement get_fill_size routine in act_gact

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_gact.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 7456325..88fbb84 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -217,6 +217,19 @@ static int tcf_gact_search(struct net *net, struct 
tc_action **a, u32 index,
return tcf_idr_search(tn, a, index);
 }
 
+static size_t tcf_gact_get_fill_size(const struct tc_action *act)
+{
+   size_t sz = nla_total_size(sizeof(struct tc_gact)); /* TCA_GACT_PARMS */
+
+#ifdef CONFIG_GACT_PROB
+   if (to_gact(act)->tcfg_ptype)
+   /* TCA_GACT_PROB */
+   sz += nla_total_size(sizeof(struct tc_gact_p));
+#endif
+
+   return sz;
+}
+
 static struct tc_action_ops act_gact_ops = {
.kind   =   "gact",
.type   =   TCA_ACT_GACT,
@@ -227,6 +240,7 @@ static struct tc_action_ops act_gact_ops = {
.init   =   tcf_gact_init,
.walk   =   tcf_gact_walker,
.lookup =   tcf_gact_search,
+   .get_fill_size  =   tcf_gact_get_fill_size,
.size   =   sizeof(struct tcf_gact),
 };
 
-- 
2.7.4

[PATCH v2 net-next 2/4] net sched actions: add new tc_action_ops callback

[Roman Mashak]

Add a new callback in tc_action_ops, it will be needed by the tc actions
to compute its size when a ADD/DELETE notification message is constructed.
This routine has to take into account optional/variable size TLVs specific
per action.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 include/net/act_api.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 88c1f99..e0a9c20 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -97,6 +97,7 @@ struct tc_action_ops {
const struct tc_action_ops *,
struct netlink_ext_ack *);
void(*stats_update)(struct tc_action *, u64, u32, u64);
+   size_t  (*get_fill_size)(const struct tc_action *act);
struct net_device *(*get_dev)(const struct tc_action *a);
 };
 
-- 
2.7.4

[PATCH v2 net-next 0/4] Fix event generation for actions batch Add/Delete mode

[Roman Mashak]

When adding or deleting a batch of entries, the kernel sends upto
TCA_ACT_MAX_PRIO entries in an event to user space. However it does not
consider that the action sizes may vary and require different skb sizes.

For example :

% cat tc-batch.sh
#!/bin/bash
TC="sudo /mnt/iproute2.git/tc/tc"

$TC actions flush action gact
for i in `seq 1 $1`;
do
   cmd="action pass index $i "
   args=$args$cmd
done
$TC actions add $args
%
% ./tc-batch.sh 32
Error: Failed to fill netlink attributes while adding TC action.
We have an error talking to the kernel
%

This patchset introduces new callback in tc_action_ops, which calculates
the action size, and passes size to tcf_add_notify()/tcf_del_notify(). The
patch fixes act_gact, and the rest of actions will be updated in the
follow-up patches.

v2:
   Restructured patches to make them bisectable.

Roman Mashak (4):
  net sched actions: update Add/Delete action API with new argument
  net sched actions: add new tc_action_ops callback
  net sched actions: calculate add/delete event message size
  net sched actions: implement get_fill_size routine in act_gact

 include/net/act_api.h |  4 +++-
 net/sched/act_api.c   | 63 ---
 net/sched/act_gact.c  | 14 
 net/sched/cls_api.c   |  3 ++-
 4 files changed, 74 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH iproute2 1/1] tc: added tc monitor description in man page

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 man/man8/tc.8 | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 5ffea37..cc94faa 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -82,12 +82,20 @@ tc \- show / manipulate traffic control settings
 \fIBLOCK_INDEX\fR
 
 .P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B monitor [ file
+\fIFILENAME\fR
+.B ]
+
+.P
 .ti 8
 .IR OPTIONS " := {"
 \fB[ -force ] -b\fR[\fIatch\fR] \fB[ filename ] \fR|
 \fB[ \fB-n\fR[\fIetns\fR] name \fB] \fR|
 \fB[ \fB-nm \fR| \fB-nam\fR[\fIes\fR] \fB] \fR|
-\fB[ \fR{ \fB-cf \fR| \fB-c\fR[\fIonf\fR] \fR} \fB[ filename ] \fB] \fR}
+\fB[ \fR{ \fB-cf \fR| \fB-c\fR[\fIonf\fR] \fR} \fB[ filename ] \fB] \fR
+\fB[ -t\fR[imestamp\fR] \fB\] \fR| \fB[ -t\fR[short\fR] \fB]\fR }
 
 .ti 8
 .IR FORMAT " := {"
@@ -616,6 +624,17 @@ link
 Only available for qdiscs and performs a replace where the node
 must exist already.
 
+.SH MONITOR
+The\fB\ tc\fR\ utility can monitor events generated by the kernel such as
+adding/deleting qdiscs, filters or actions, or modifying existing ones.
+
+The following command is available for\fB\ monitor\fR\ :
+.TP
+\fBfile\fR
+If the file option is given, the \fBtc\fR does not listen to kernel events, 
but opens
+the given file and dumps its contents. The file has to be in binary
+format and contain netlink messages.
+
 .SH OPTIONS
 
 .TP
@@ -653,6 +672,16 @@ to
 specifies path to the config file. This option is used in conjunction with 
other options (e.g.
 .BR -nm ")."
 
+.TP
+.BR "\-t", " \-timestamp"
+When\fB\ tc monitor\fR\ runs, print timestamp before the event message in 
format:
+   Timestamp:   usec
+
+.TP
+.BR "\-ts", " \-tshort"
+When\fB\ tc monitor\fR\ runs, prints short timestamp before the event message 
in format:
+   [--T.]
+
 .SH FORMAT
 The show command has additional formatting options:
 
-- 
2.7.4

Re: [PATCH net-next 2/5] net sched actions: add new tc_action_ops callback

[Roman Mashak]

David Miller <da...@davemloft.net> writes:

> From: Roman Mashak <m...@mojatatu.com>
> Date: Fri,  2 Mar 2018 17:01:40 -0500
>
>> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>> index acac92a..6f3307f 100644
>> --- a/net/sched/act_api.c
>> +++ b/net/sched/act_api.c
>> @@ -136,6 +136,14 @@ static size_t tcf_action_full_attrs_size(size_t sz)
>>  + sz;
>>  }
>>  
>> +static size_t tcf_action_fill_size(const struct tc_action *act)
>> +{
>> +if (act->ops->get_fill_size)
>> +return act->ops->get_fill_size(act) +
>> +tcf_action_shared_attrs_size(act);
>> +return 0;
>> +}
>> +
>
> Again, a static functions with no users generates compiler warnings.

Thanks, will fix in v2.

Re: [PATCH net-next 1/5] net sched actions: routines to calculate common TLVs size

[Roman Mashak]

David Miller <da...@davemloft.net> writes:

> From: Roman Mashak <m...@mojatatu.com>
> Date: Fri,  2 Mar 2018 17:01:39 -0500
>
>> Introduce routine to calculate size of the common tc netlink attributes,
>> and another helper routine to get the full message size including netlink
>> header and service header.
>> 
>> Signed-off-by: Roman Mashak <m...@mojatatu.com>
>
> Adding the helpers as static functions, with no users, makes this
> series non-bisectable because the compiler will warn about the unused
> functions at this point in the series.

Thank you. I will fix in v2.

[PATCH net-next 1/1] net sched actions: corrected extack message

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 1f65d6a..a54fa7b 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1083,7 +1083,7 @@ tcf_add_notify(struct net *net, struct nlmsghdr *n, 
struct list_head *actions,
 
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
 RTM_NEWACTION, 0, 0) <= 0) {
-   NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while 
deleting TC action");
+   NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while 
adding TC action");
kfree_skb(skb);
return -EINVAL;
}
-- 
2.7.4

[PATCH net-next 5/5] net sched actions: implement get_fill_size routine in act_police

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_police.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 51fe4fe..d4b4b15 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -314,6 +314,13 @@ static int tcf_police_search(struct net *net, struct 
tc_action **a, u32 index,
return tcf_idr_search(tn, a, index);
 }
 
+static size_t tcf_police_get_fill_size(const struct tc_action *act)
+{
+   return nla_total_size(sizeof(struct tc_police)) /* TCA_POLICE_TBF */
+   + nla_total_size(sizeof(u32)) /* TCA_POLICE_RESULT */
+   + nla_total_size(sizeof(u32)); /* TCA_POLICE_AVRATE */
+}
+
 MODULE_AUTHOR("Alexey Kuznetsov");
 MODULE_DESCRIPTION("Policing actions");
 MODULE_LICENSE("GPL");
@@ -327,6 +334,7 @@ static struct tc_action_ops act_police_ops = {
.init   =   tcf_act_police_init,
.walk   =   tcf_act_police_walker,
.lookup =   tcf_police_search,
+   .get_fill_size  =   tcf_police_get_fill_size,
.size   =   sizeof(struct tcf_police),
 };
 
-- 
2.7.4

[PATCH net-next 4/5] net sched actions: implement get_fill_size routine in act_gact

[Roman Mashak]

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_gact.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 7456325..88fbb84 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -217,6 +217,19 @@ static int tcf_gact_search(struct net *net, struct 
tc_action **a, u32 index,
return tcf_idr_search(tn, a, index);
 }
 
+static size_t tcf_gact_get_fill_size(const struct tc_action *act)
+{
+   size_t sz = nla_total_size(sizeof(struct tc_gact)); /* TCA_GACT_PARMS */
+
+#ifdef CONFIG_GACT_PROB
+   if (to_gact(act)->tcfg_ptype)
+   /* TCA_GACT_PROB */
+   sz += nla_total_size(sizeof(struct tc_gact_p));
+#endif
+
+   return sz;
+}
+
 static struct tc_action_ops act_gact_ops = {
.kind   =   "gact",
.type   =   TCA_ACT_GACT,
@@ -227,6 +240,7 @@ static struct tc_action_ops act_gact_ops = {
.init   =   tcf_gact_init,
.walk   =   tcf_gact_walker,
.lookup =   tcf_gact_search,
+   .get_fill_size  =   tcf_gact_get_fill_size,
.size   =   sizeof(struct tcf_gact),
 };
 
-- 
2.7.4

[PATCH net-next 2/5] net sched actions: add new tc_action_ops callback

[Roman Mashak]

Add a new callback in tc_action_ops, it will be needed by the tc actions
to compute its size when a ADD/DELETE notification message is constructed.
This routine has to take into account optional/variable size TLVs specific
per action.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 include/net/act_api.h | 1 +
 net/sched/act_api.c   | 8 
 2 files changed, 9 insertions(+)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 9c2f226..0a56465 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -97,6 +97,7 @@ struct tc_action_ops {
const struct tc_action_ops *,
struct netlink_ext_ack *);
void(*stats_update)(struct tc_action *, u64, u32, u64);
+   size_t  (*get_fill_size)(const struct tc_action *act);
struct net_device *(*get_dev)(const struct tc_action *a);
 };
 
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index acac92a..6f3307f 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -136,6 +136,14 @@ static size_t tcf_action_full_attrs_size(size_t sz)
+ sz;
 }
 
+static size_t tcf_action_fill_size(const struct tc_action *act)
+{
+   if (act->ops->get_fill_size)
+   return act->ops->get_fill_size(act) +
+   tcf_action_shared_attrs_size(act);
+   return 0;
+}
+
 static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
   struct netlink_callback *cb)
 {
-- 
2.7.4

[PATCH net-next 3/5] net sched actions: calculate add/delete event message size

[Roman Mashak]

Update add/delete action logic to have the size for event messages,
the size is passed to tcf_add_notify() and tcf_del_notify().

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 include/net/act_api.h |  3 ++-
 net/sched/act_api.c   | 26 ++
 net/sched/cls_api.c   |  3 ++-
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 0a56465..e0a9c20 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -167,7 +167,8 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action 
**actions,
int nr_actions, struct tcf_result *res);
 int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
-   struct list_head *actions, struct netlink_ext_ack *extack);
+   struct list_head *actions, size_t *attr_size,
+   struct netlink_ext_ack *extack);
 struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
char *name, int ovr, int bind,
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6f3307f..097ca07 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -776,10 +776,12 @@ static void cleanup_a(struct list_head *actions, int ovr)
 
 int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
-   struct list_head *actions, struct netlink_ext_ack *extack)
+   struct list_head *actions, size_t *attr_size,
+   struct netlink_ext_ack *extack)
 {
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+   size_t sz = 0;
int err;
int i;
 
@@ -795,11 +797,14 @@ int tcf_action_init(struct net *net, struct tcf_proto 
*tp, struct nlattr *nla,
goto err;
}
act->order = i;
+   sz += tcf_action_fill_size(act);
if (ovr)
act->tcfa_refcnt++;
list_add_tail(>list, actions);
}
 
+   *attr_size = tcf_action_full_attrs_size(sz);
+
/* Remove the temp refcnt which was necessary to protect against
 * destroying an existing action which was being replaced
 */
@@ -1029,12 +1034,13 @@ static int tca_action_flush(struct net *net, struct 
nlattr *nla,
 
 static int
 tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, struct netlink_ext_ack *extack)
+  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
 {
int ret;
struct sk_buff *skb;
 
-   skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
+   GFP_KERNEL);
if (!skb)
return -ENOBUFS;
 
@@ -1067,6 +1073,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct 
nlmsghdr *n,
int i, ret;
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+   size_t attr_size = 0;
LIST_HEAD(actions);
 
ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
@@ -1088,13 +1095,14 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
goto err;
}
act->order = i;
+   attr_size += tcf_action_fill_size(act);
list_add_tail(>list, );
}
 
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, , event, extack);
else { /* delete */
-   ret = tcf_del_notify(net, n, , portid, extack);
+   ret = tcf_del_notify(net, n, , portid, attr_size, 
extack);
if (ret)
goto err;
return ret;
@@ -1107,12 +1115,13 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
 
 static int
 tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
-  u32 portid, struct netlink_ext_ack *extack)
+  u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
 {
struct sk_buff *skb;
int err = 0;
 
-   skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   skb = alloc_skb(attr_size <= NLMSG_GOODSIZE ? NLMSG_GOODSIZE : 
attr_size,
+   GFP_KERNEL);
if (!skb)
return -ENOBUFS;
 
@@ -1134,15 +1143,16 @@ static int tcf_action_add(struct net *net, struct 
nlattr *nla,
  struct nlmsghdr *n, u32 portid, int ovr,
  struct netlink_ext_ack *extack)
 {
+   size_t attr_size = 0;
int ret = 0;
LIST_H

[PATCH net-next 0/5] Fix event generation for actions batch Add/Delete mode

[Roman Mashak]

When adding or deleting a batch of entries, the kernel sends upto
TCA_ACT_MAX_PRIO entries in an event to user space. However it does not
consider that the action sizes may vary and require different skb sizes.

For example :

% cat tc-batch.sh
#!/bin/bash
TC="sudo /mnt/iproute2.git/tc/tc"

$TC actions flush action gact
for i in `seq 1 $1`;
do
   cmd="action pass index $i "
   args=$args$cmd
done
$TC actions add $args
%
% ./tc-batch.sh 32
Error: Failed to fill netlink attributes while deleting TC action.
We have an error talking to the kernel
%

This patchset introduces new callback in tc_action_ops, which calculates
the action size, and passes size to tcf_add_notify()/tcf_del_notify(). The
patch fixes act_gact and act_police, and the rest of actions will be
updated in the follow-up patches.

Roman Mashak (5):
  net sched actions: routines to calculate common TLVs size
  net sched actions: add new tc_action_ops callback
  net sched actions: calculate add/delete event message size.
  net sched actions: implement get_fill_size routine in act_gact
  net sched actions: implement get_fill_size routine in act_police

 include/net/act_api.h  |  6 -
 net/sched/act_api.c| 62 +++---
 net/sched/act_gact.c   | 18 +++
 net/sched/act_police.c |  9 
 net/sched/cls_api.c|  3 ++-
 5 files changed, 88 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH net-next 1/5] net sched actions: routines to calculate common TLVs size

[Roman Mashak]

Introduce routine to calculate size of the common tc netlink attributes,
and another helper routine to get the full message size including netlink
header and service header.

Signed-off-by: Roman Mashak <m...@mojatatu.com>
---
 net/sched/act_api.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 1f65d6a..acac92a 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -109,6 +109,33 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool 
strict)
 }
 EXPORT_SYMBOL(__tcf_idr_release);
 
+static size_t tcf_action_shared_attrs_size(const struct tc_action *act)
+{
+   u32 cookie_len = 0;
+
+   if (act->act_cookie)
+   cookie_len = nla_total_size(act->act_cookie->len);
+
+   return  nla_total_size(0) /* action number nested */
+   + nla_total_size(IFNAMSIZ) /* TCA_ACT_KIND */
+   + cookie_len /* TCA_ACT_COOKIE */
+   + nla_total_size(0) /* TCA_ACT_STATS nested */
+   /* TCA_STATS_BASIC */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_basic))
+   /* TCA_STATS_QUEUE */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_queue))
+   + nla_total_size(0) /* TCA_OPTIONS nested */
+   + nla_total_size(sizeof(struct tcf_t)); /* TCA_GACT_TM */
+}
+
+static size_t tcf_action_full_attrs_size(size_t sz)
+{
+   return NLMSG_HDRLEN /* struct nlmsghdr */
+   + sizeof(struct tcamsg)
+   + nla_total_size(0) /* TCA_ACT_TAB nested */
+   + sz;
+}
+
 static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
   struct netlink_callback *cb)
 {
-- 
2.7.4