Re: [RFC net-next 0/6] xdp: make stack perform remove and tests

2017-11-25 Thread Jiri Pirko
Sun, Nov 26, 2017 at 02:43:04AM CET, jakub.kicin...@netronome.com wrote:
>On Sat, 25 Nov 2017 17:02:09 -0800, Alexei Starovoitov wrote:
>> On Thu, Nov 23, 2017 at 06:36:07PM -0800, Jakub Kicinski wrote:
>> > Hi!
>> > 
>> > The purpose of this series is to add a software model of BPF offloads
>> > to make it easier for everyone to test them and make some of the more
>> > arcane rules and assumptions more clear.
>> > 
>> > The series starts with 3 patches aiming to make XDP handling in the
>> > drivers less error prone.  Currently driver authors have to remember
>> > to free XDP programs if XDP is active during unregister.  With this
>> > series the core will disable XDP on its own.  It will take place
>> > after close, drivers are not expected to perform reconfiguration
>> > when disabling XDP on a downed device.
>> > 
>> > Next two patches add the software netdev driver.  Last but not least
>> > there is a python test which exercises all the corner cases which
>> > came to my mind.
>> > 
>> > Test needs to be run as root.  It will print basic information to
>> > stdout, but can also create a more detailed log of all commands
>> > when --log option is passed.  Log is in Emacs Org-mode format.
>> > 
>> >   ./tools/testing/selftests/bpf/test_offload.py --log /tmp/log
>> > 
>> > Something I'm still battling with, and would appreciate help of
>> > wiser people is that occasionally during the test something makes
>> > the refcount of init_net drop to 0 :S  I tried to create a simple
>> > reproducer, but seems like just running the script in the loop is
>> > the easiest way to go...  Could it have something to do with the
>> > recent TC work?  The driver is pretty simple and never touches
>> > ref counts.  The only slightly unusual thing is that the BPF code
>> > sleeps for a bit on remove in the netdev notifier.  
>> 
>> I like the direction.
>> Patch 3 in particular with auto prog_put is a great idea!
>> Patch 4 - if you want to do dual gpl+bsd. It's ok and really your call as an 
>> author.

Sure. I was just curious if it is really necessary. That's all.


>> Patch 5 - I'm only not excited about debugfs. Can you do the same with 
>> tracepoints?
>
>I would prefer to stick to DebugFS, if you don't mind.  Some of the
>knobs are writeable.  I look at DebugFS as a back door.  We throw
>standard API commands at the device and want to inspect its internal
>state.  And cross check the standard reporting APIs.  As such DebugFS
>is completely inappropriate but for API-test-drivers.

I agree. This is one of few exceptions where debugfs is good to be used.


>
>On that topic - is there a way to trigger and wait for an rcu barrier
>from user space?  Quentin pointed out that selftest is shaky unless run
>with RCU debug enabled.  To wait for deletion of programs I need a
>barrier :S
>
>> Patch 6 - python 3 is a requirement? or can work with 2.7 ? I don't mind if 
>> it's 3+ only.
>> Just trying to understand the dependencies.
>
>Am a python noob so I defaulted to version 3 and proceeded to look
>things I needed up in the python 3 docs.  Which perhaps resulted in
>using more python3 only features than necessary...  
>
>Upon a quick inspection, the only python3 feature I seem to be using 
>is the Popen.args member.
>
>> and thanks for adding it to selftests.
>
>Thanks for the comments!


Re: [PATCHv2 2/2] userns: control capabilities of some user namespaces

2017-11-25 Thread Serge E. Hallyn
Quoting Mahesh Bandewar (mah...@bandewar.net):
> From: Mahesh Bandewar 
> 
> With this new notion of "controlled" user-namespaces, the controlled
> user-namespaces are marked at the time of their creation while the
> capabilities of processes that belong to them are controlled using the
> global mask.
> 
> Init-user-ns is always uncontrolled and a process that has SYS_ADMIN
> that belongs to uncontrolled user-ns can create another (child) user-
> namespace that is uncontrolled. Any other process (that either does
> not have SYS_ADMIN or belongs to a controlled user-ns) can only
> create a user-ns that is controlled.
> 
> global-capability-whitelist (controlled_userns_caps_whitelist) is used
> at the capability check-time and keeps the semantics for the processes
> that belong to uncontrolled user-ns as it is. Processes that belong to
> controlled user-ns however are subjected to different checks-
> 
>(a) if the capability in question is controlled and process belongs
>to controlled user-ns, then it's always denied.
>(b) if the capability in question is NOT controlled then fall back
>to the traditional check.
> 
> Signed-off-by: Mahesh Bandewar 

Acked-by: Serge Hallyn 

Although a few comment addition requests below:

> ---
> v2:
>   Don't recalculate user-ns flags for every setns() call.
> v1:
>   Initial submission.
> 
>  include/linux/capability.h |  1 +
>  include/linux/user_namespace.h | 20 
>  kernel/capability.c|  5 +
>  kernel/user_namespace.c|  4 
>  security/commoncap.c   |  8 
>  5 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 7d79a4689625..a1fd9e460379 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -251,6 +251,7 @@ extern bool ptracer_capable(struct task_struct *tsk, 
> struct user_namespace *ns);
>  extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct 
> cpu_vfs_cap_data *cpu_caps);
>  int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
>void __user *buff, size_t *lenp, loff_t *ppos);

Here and at the definition below, please add a comment explaining
that a controlled cap is defined as not being in the sysctl.

> +bool is_capability_controlled(int cap);
>  
>  extern int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t 
> size);
>  
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 3fe714da7f5a..647f825c7b5f 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -23,6 +23,7 @@ struct uid_gid_map {/* 64 bytes -- 1 cache line */
>  };
>  
>  #define USERNS_SETGROUPS_ALLOWED 1UL
> +#define USERNS_CONTROLLED 2UL
>  
>  #define USERNS_INIT_FLAGS USERNS_SETGROUPS_ALLOWED
>  
> @@ -103,6 +104,16 @@ static inline void put_user_ns(struct user_namespace *ns)
>   __put_user_ns(ns);
>  }
>  

Please add a comment explaining that a controlled ns
is one created by a user which did not have CAP_SYS_ADMIN
(or descended from such an ns).

> +static inline bool is_user_ns_controlled(const struct user_namespace *ns)
> +{
> + return ns->flags & USERNS_CONTROLLED;
> +}
> +
> +static inline void mark_user_ns_controlled(struct user_namespace *ns)
> +{
> + ns->flags |= USERNS_CONTROLLED;
> +}
> +
>  struct seq_operations;
>  extern const struct seq_operations proc_uid_seq_operations;
>  extern const struct seq_operations proc_gid_seq_operations;
> @@ -161,6 +172,15 @@ static inline struct ns_common *ns_get_owner(struct 
> ns_common *ns)
>  {
>   return ERR_PTR(-EPERM);
>  }
> +
> +static inline bool is_user_ns_controlled(const struct user_namespace *ns)
> +{
> + return false;
> +}
> +
> +static inline void mark_user_ns_controlled(struct user_namespace *ns)
> +{
> +}
>  #endif
>  
>  #endif /* _LINUX_USER_H */
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 4a859b7d4902..bffe249922de 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -511,6 +511,11 @@ bool ptracer_capable(struct task_struct *tsk, struct 
> user_namespace *ns)
>  }
>  
>  /* Controlled-userns capabilities routines */
> +bool is_capability_controlled(int cap)
> +{
> + return !cap_raised(controlled_userns_caps_whitelist, cap);
> +}
> +
>  #ifdef CONFIG_SYSCTL
>  int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
>void __user *buff, size_t *lenp, loff_t *ppos)
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index c490f1e4313b..600c7dcb9ff7 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -139,6 +139,10 @@ int create_user_ns(struct cred *new)
>   goto fail_keyring;
>  
>   set_cred_user_ns(new, ns);
> + if (!ns_capable(parent_ns, CAP_SYS_ADMIN) ||
> + 

Re: [PATCHv2 1/2] capability: introduce sysctl for controlled user-ns capability whitelist

2017-11-25 Thread Serge E. Hallyn
Quoting Mahesh Bandewar (mah...@bandewar.net):
> From: Mahesh Bandewar 
> 
> Add a sysctl variable kernel.controlled_userns_caps_whitelist. This
> takes input as capability mask expressed as two comma separated hex
> u32 words. The mask, however, is stored in kernel as kernel_cap_t type.
> 
> Any capabilities that are not part of this mask will be controlled and
> will not be allowed to processes in controlled user-ns.
> 
> Signed-off-by: Mahesh Bandewar 

Acked-by: Serge Hallyn 

> ---
> v2:
>   Rebase
> v1:
>   Initial submission
> 
>  Documentation/sysctl/kernel.txt | 21 ++
>  include/linux/capability.h  |  3 +++
>  kernel/capability.c | 47 
> +
>  kernel/sysctl.c |  5 +
>  4 files changed, 76 insertions(+)
> 
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 694968c7523c..a1d39dbae847 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -25,6 +25,7 @@ show up in /proc/sys/kernel:
>  - bootloader_version  [ X86 only ]
>  - callhome[ S390 only ]
>  - cap_last_cap
> +- controlled_userns_caps_whitelist
>  - core_pattern
>  - core_pipe_limit
>  - core_uses_pid
> @@ -187,6 +188,26 @@ CAP_LAST_CAP from the kernel.
>  
>  ==
>  
> +controlled_userns_caps_whitelist
> +
> +Capability mask that is whitelisted for "controlled" user namespaces.
> +Any capability that is missing from this mask will not be allowed to
> +any process that is attached to a controlled-userns. e.g. if CAP_NET_RAW
> +is not part of this mask, then processes running inside any controlled
> +userns's will not be allowed to perform action that needs CAP_NET_RAW
> +capability. However, processes that are attached to a parent user-ns
> +hierarchy that is *not* controlled and has CAP_NET_RAW can continue
> +performing those actions. User-namespaces are marked "controlled" at
> +the time of their creation based on the capabilities of the creator.
> +A process that does not have CAP_SYS_ADMIN will create user-namespaces
> +that are controlled.
> +
> +The value is expressed as two comma separated hex words (u32). This
> +sysctl is avaialble in init-ns and users with CAP_SYS_ADMIN in init-ns
> +are allowed to make changes.
> +
> +==
> +
>  core_pattern:
>  
>  core_pattern is used to specify a core dumpfile pattern name.
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index f640dcbc880c..7d79a4689625 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -14,6 +14,7 @@
>  #define _LINUX_CAPABILITY_H
>  
>  #include 
> +#include 
>  
>  
>  #define _KERNEL_CAPABILITY_VERSION _LINUX_CAPABILITY_VERSION_3
> @@ -248,6 +249,8 @@ extern bool ptracer_capable(struct task_struct *tsk, 
> struct user_namespace *ns);
>  
>  /* audit system wants to get cap info from files as well */
>  extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct 
> cpu_vfs_cap_data *cpu_caps);
> +int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
> +  void __user *buff, size_t *lenp, loff_t *ppos);
>  
>  extern int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t 
> size);
>  
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 1e1c0236f55b..4a859b7d4902 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -29,6 +29,8 @@ EXPORT_SYMBOL(__cap_empty_set);
>  
>  int file_caps_enabled = 1;
>  
> +kernel_cap_t controlled_userns_caps_whitelist = CAP_FULL_SET;
> +
>  static int __init file_caps_disable(char *str)
>  {
>   file_caps_enabled = 0;
> @@ -507,3 +509,48 @@ bool ptracer_capable(struct task_struct *tsk, struct 
> user_namespace *ns)
>   rcu_read_unlock();
>   return (ret == 0);
>  }
> +
> +/* Controlled-userns capabilities routines */
> +#ifdef CONFIG_SYSCTL
> +int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
> +  void __user *buff, size_t *lenp, loff_t *ppos)
> +{
> + DECLARE_BITMAP(caps_bitmap, CAP_LAST_CAP);
> + struct ctl_table caps_table;
> + char tbuf[NAME_MAX];
> + int ret;
> +
> + ret = bitmap_from_u32array(caps_bitmap, CAP_LAST_CAP,
> +controlled_userns_caps_whitelist.cap,
> +_KERNEL_CAPABILITY_U32S);
> + if (ret != CAP_LAST_CAP)
> + return -1;
> +
> + scnprintf(tbuf, NAME_MAX, "%*pb", CAP_LAST_CAP, caps_bitmap);
> +
> + caps_table.data = tbuf;
> + caps_table.maxlen = NAME_MAX;
> + caps_table.mode = table->mode;
> + ret = proc_dostring(_table, write, buff, lenp, ppos);
> + if (ret)
> + return ret;
> + if (write) {
> + kernel_cap_t tmp;
> +
> +

Dear Friend I need your partnership to claim,

2017-11-25 Thread Mr. Paul Harry
Hello My Dear Friend, with due respect to your person and much sincerity of 
purpose, It’s my pleasure to write you today, I am Mr. Paul Harry, I work in a 
bank. and I get your contact from internet search I hope that you will not 
expose or betray this trust and confident that am about to repose in you for 
the benefit of our both families am in need of your help as a foreigner to 
transfer Ten Million Three Hundred and Fifty thousand Usd ($10.350 Million) 
into your account, risk is completely %100 free.

Regards
Mr. Paul Harry.


Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE

2017-11-25 Thread Alexei Starovoitov

On 11/24/17 12:28 AM, Peter Zijlstra wrote:

On Thu, Nov 23, 2017 at 10:31:29PM -0800, Alexei Starovoitov wrote:

unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include 

struct S {
  unsigned long long a;
} s;

struct U {
  unsigned long long a;
} u;

int main()
{
printf("%d, %d\n", sizeof(unsigned long long),
   __alignof__(unsigned long long));
printf("%d, %d\n", sizeof(s), __alignof__(s));
printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4


*blink* how is that even correct? I understood the spec to say the
alignment of composite types should be the max alignment of any of its
member types (otherwise it cannot guarantee the alignment of its
members).


so we have to use __aligned_u64 in uapi.


Ideally yes, but effectively it most often doesn't matter.


Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.


I don't understand the reasoning why you cannot use them. Even if they
are not naturally aligned on x86_32, why would it matter?

x86_32 needs two loads in any case, but there is no concurrency, so
split loads is not a problem. Add to that that 'intptr_t' on ILP32
is in fact only a single u32 and thus the other u32 will always be 0.

So yes, alignment is screwy, but I really don't see who cares and why it
would matter in practise.


If we were poking into 'struct perf_event_attr __user *uptr'
directly like get|put_user(.., >config)
then 32-bit user space with 4-byte aligned u64s would cause
64-bit kernel to trap on archs like sparc.
But in this case you're right. We can use config[12] as-is, since these
u64 fields are passing the value one way only (into the kernel) and
we do full perf_copy_attr() first and all further accesses are from
copied structure and u64_to_user_ptr(event->attr.config) will be fine.

Do you mind we do
union {
 __u64 file_path;
 __u64 func_name;
 __u64 config;
};
and similar with config1 ?
Or prefer that we use 'config/config1' to store string+offset there?
I think config/config1 is cleaner than config1/config2



Re: [RFC net-next 0/6] xdp: make stack perform remove and tests

2017-11-25 Thread Jakub Kicinski
On Sat, 25 Nov 2017 17:02:09 -0800, Alexei Starovoitov wrote:
> On Thu, Nov 23, 2017 at 06:36:07PM -0800, Jakub Kicinski wrote:
> > Hi!
> > 
> > The purpose of this series is to add a software model of BPF offloads
> > to make it easier for everyone to test them and make some of the more
> > arcane rules and assumptions more clear.
> > 
> > The series starts with 3 patches aiming to make XDP handling in the
> > drivers less error prone.  Currently driver authors have to remember
> > to free XDP programs if XDP is active during unregister.  With this
> > series the core will disable XDP on its own.  It will take place
> > after close, drivers are not expected to perform reconfiguration
> > when disabling XDP on a downed device.
> > 
> > Next two patches add the software netdev driver.  Last but not least
> > there is a python test which exercises all the corner cases which
> > came to my mind.
> > 
> > Test needs to be run as root.  It will print basic information to
> > stdout, but can also create a more detailed log of all commands
> > when --log option is passed.  Log is in Emacs Org-mode format.
> > 
> >   ./tools/testing/selftests/bpf/test_offload.py --log /tmp/log
> > 
> > Something I'm still battling with, and would appreciate help of
> > wiser people is that occasionally during the test something makes
> > the refcount of init_net drop to 0 :S  I tried to create a simple
> > reproducer, but seems like just running the script in the loop is
> > the easiest way to go...  Could it have something to do with the
> > recent TC work?  The driver is pretty simple and never touches
> > ref counts.  The only slightly unusual thing is that the BPF code
> > sleeps for a bit on remove in the netdev notifier.  
> 
> I like the direction.
> Patch 3 in particular with auto prog_put is a great idea!
> Patch 4 - if you want to do dual gpl+bsd. It's ok and really your call as an 
> author.
> Patch 5 - I'm only not excited about debugfs. Can you do the same with 
> tracepoints?

I would prefer to stick to DebugFS, if you don't mind.  Some of the
knobs are writeable.  I look at DebugFS as a back door.  We throw
standard API commands at the device and want to inspect its internal
state.  And cross check the standard reporting APIs.  As such DebugFS
is completely inappropriate but for API-test-drivers.

On that topic - is there a way to trigger and wait for an rcu barrier
from user space?  Quentin pointed out that selftest is shaky unless run
with RCU debug enabled.  To wait for deletion of programs I need a
barrier :S

> Patch 6 - python 3 is a requirement? or can work with 2.7 ? I don't mind if 
> it's 3+ only.
> Just trying to understand the dependencies.

Am a python noob so I defaulted to version 3 and proceeded to look
things I needed up in the python 3 docs.  Which perhaps resulted in
using more python3 only features than necessary...  

Upon a quick inspection, the only python3 feature I seem to be using 
is the Popen.args member.

> and thanks for adding it to selftests.

Thanks for the comments!


Re: [RFC net-next 0/6] xdp: make stack perform remove and tests

2017-11-25 Thread Alexei Starovoitov
On Thu, Nov 23, 2017 at 06:36:07PM -0800, Jakub Kicinski wrote:
> Hi!
> 
> The purpose of this series is to add a software model of BPF offloads
> to make it easier for everyone to test them and make some of the more
> arcane rules and assumptions more clear.
> 
> The series starts with 3 patches aiming to make XDP handling in the
> drivers less error prone.  Currently driver authors have to remember
> to free XDP programs if XDP is active during unregister.  With this
> series the core will disable XDP on its own.  It will take place
> after close, drivers are not expected to perform reconfiguration
> when disabling XDP on a downed device.
> 
> Next two patches add the software netdev driver.  Last but not least
> there is a python test which exercises all the corner cases which
> came to my mind.
> 
> Test needs to be run as root.  It will print basic information to
> stdout, but can also create a more detailed log of all commands
> when --log option is passed.  Log is in Emacs Org-mode format.
> 
>   ./tools/testing/selftests/bpf/test_offload.py --log /tmp/log
> 
> Something I'm still battling with, and would appreciate help of
> wiser people is that occasionally during the test something makes
> the refcount of init_net drop to 0 :S  I tried to create a simple
> reproducer, but seems like just running the script in the loop is
> the easiest way to go...  Could it have something to do with the
> recent TC work?  The driver is pretty simple and never touches
> ref counts.  The only slightly unusual thing is that the BPF code
> sleeps for a bit on remove in the netdev notifier.

I like the direction.
Patch 3 in particular with auto prog_put is a great idea!
Patch 4 - if you want to do dual gpl+bsd. It's ok and really your call as an 
author.
Patch 5 - I'm only not excited about debugfs. Can you do the same with 
tracepoints?
Patch 6 - python 3 is a requirement? or can work with 2.7 ? I don't mind if 
it's 3+ only.
Just trying to understand the dependencies.
and thanks for adding it to selftests.



Re: [PATCH] dsa: dsa2: fix compile error for !CONFIG_OF

2017-11-25 Thread Florian Fainelli


On 11/25/2017 06:54 AM, David Miller wrote:
> 
> Please repost this without the huge kernel config :-/

This does not appear to be necessary, this commit
96c623e51f1c40bf524decc48c6fac7ce5dd41f7 ("of: add
of_property_read_variable_* dummy helpers") fixes this already.
-- 
Florian


[PATCH] NFC: use timer_setup() helper

2017-11-25 Thread Vasyl Gomonovych
Use timer_setup function instead of initializing timer with the
function and data fields.

net/nfc/core.c:1097:2-12: Use setup_timer function for function on line 1098
net/nfc/hci/core.c:1007:1-11: Use setup_timer function for function on line 1008
net/nfc/hci/llc_shdlc.c:766:1-11: Use setup_timer function for function on line 
767
net/nfc/hci/llc_shdlc.c:766:1-11: Use setup_timer function for function on line 
767
net/nfc/llcp_core.c:1576:1-11: Use setup_timer function for function on line 
1577

Generated by: scripts/coccinelle/api/setup_timer.cocci

Signed-off-by: Vasyl Gomonovych 
---
 net/nfc/core.c  |  8 +++-
 net/nfc/hci/core.c  |  8 +++-
 net/nfc/hci/llc_shdlc.c | 26 +-
 net/nfc/llcp_core.c | 16 ++--
 4 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/net/nfc/core.c b/net/nfc/core.c
index 5cf33df888c3..b220b0406b57 100644
--- a/net/nfc/core.c
+++ b/net/nfc/core.c
@@ -1015,9 +1015,9 @@ static void nfc_check_pres_work(struct work_struct *work)
device_unlock(>dev);
 }
 
-static void nfc_check_pres_timeout(unsigned long data)
+static void nfc_check_pres_timeout(struct timer_list *t)
 {
-   struct nfc_dev *dev = (struct nfc_dev *)data;
+   struct nfc_dev *dev = from_timer(dev, t, check_pres_timer);
 
schedule_work(>check_pres_work);
 }
@@ -1094,9 +1094,7 @@ struct nfc_dev *nfc_allocate_device(struct nfc_ops *ops,
dev->targets_generation = 1;
 
if (ops->check_presence) {
-   init_timer(>check_pres_timer);
-   dev->check_pres_timer.data = (unsigned long)dev;
-   dev->check_pres_timer.function = nfc_check_pres_timeout;
+   timer_setup(>check_pres_timer, nfc_check_pres_timeout, 0);
 
INIT_WORK(>check_pres_work, nfc_check_pres_work);
}
diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index b740fef0acc5..ac8030c4bcf8 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -428,9 +428,9 @@ void nfc_hci_event_received(struct nfc_hci_dev *hdev, u8 
pipe, u8 event,
nfc_hci_driver_failure(hdev, r);
 }
 
-static void nfc_hci_cmd_timeout(unsigned long data)
+static void nfc_hci_cmd_timeout(struct timer_list *t)
 {
-   struct nfc_hci_dev *hdev = (struct nfc_hci_dev *)data;
+   struct nfc_hci_dev *hdev = from_timer(hdev, t, cmd_timer);
 
schedule_work(>msg_tx_work);
 }
@@ -1004,9 +1004,7 @@ int nfc_hci_register_device(struct nfc_hci_dev *hdev)
 
INIT_WORK(>msg_tx_work, nfc_hci_msg_tx_work);
 
-   init_timer(>cmd_timer);
-   hdev->cmd_timer.data = (unsigned long)hdev;
-   hdev->cmd_timer.function = nfc_hci_cmd_timeout;
+   timer_setup(>cmd_timer, nfc_hci_cmd_timeout, 0);
 
skb_queue_head_init(>rx_hcp_frags);
 
diff --git a/net/nfc/hci/llc_shdlc.c b/net/nfc/hci/llc_shdlc.c
index 17e59a009ce6..fe988936ad92 100644
--- a/net/nfc/hci/llc_shdlc.c
+++ b/net/nfc/hci/llc_shdlc.c
@@ -580,27 +580,27 @@ static void llc_shdlc_handle_send_queue(struct llc_shdlc 
*shdlc)
}
 }
 
-static void llc_shdlc_connect_timeout(unsigned long data)
+static void llc_shdlc_connect_timeout(struct timer_list *t)
 {
-   struct llc_shdlc *shdlc = (struct llc_shdlc *)data;
+   struct llc_shdlc *shdlc = from_timer(shdlc, t, connect_timer);
 
pr_debug("\n");
 
schedule_work(>sm_work);
 }
 
-static void llc_shdlc_t1_timeout(unsigned long data)
+static void llc_shdlc_t1_timeout(struct timer_list *t)
 {
-   struct llc_shdlc *shdlc = (struct llc_shdlc *)data;
+   struct llc_shdlc *shdlc = from_timer(shdlc, t, t1_timer);
 
pr_debug("SoftIRQ: need to send ack\n");
 
schedule_work(>sm_work);
 }
 
-static void llc_shdlc_t2_timeout(unsigned long data)
+static void llc_shdlc_t2_timeout(struct timer_list *t)
 {
-   struct llc_shdlc *shdlc = (struct llc_shdlc *)data;
+   struct llc_shdlc *shdlc = from_timer(shdlc, t, t2_timer);
 
pr_debug("SoftIRQ: need to retransmit\n");
 
@@ -763,17 +763,9 @@ static void *llc_shdlc_init(struct nfc_hci_dev *hdev, 
xmit_to_drv_t xmit_to_drv,
mutex_init(>state_mutex);
shdlc->state = SHDLC_DISCONNECTED;
 
-   init_timer(>connect_timer);
-   shdlc->connect_timer.data = (unsigned long)shdlc;
-   shdlc->connect_timer.function = llc_shdlc_connect_timeout;
-
-   init_timer(>t1_timer);
-   shdlc->t1_timer.data = (unsigned long)shdlc;
-   shdlc->t1_timer.function = llc_shdlc_t1_timeout;
-
-   init_timer(>t2_timer);
-   shdlc->t2_timer.data = (unsigned long)shdlc;
-   shdlc->t2_timer.function = llc_shdlc_t2_timeout;
+   timer_setup(>connect_timer, llc_shdlc_connect_timeout, 0);
+   timer_setup(>t1_timer, llc_shdlc_t1_timeout, 0);
+   timer_setup(>t2_timer, llc_shdlc_t2_timeout, 0);
 
shdlc->w = SHDLC_MAX_WINDOW;
shdlc->srej_support = SHDLC_SREJ_SUPPORT;
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 

[PATCH RFC] veth: make veth aware of gso buffer size

2017-11-25 Thread Solio Sarabia
GSO buffer size supported by underlying devices is not propagated to
veth. In high-speed connections with hw TSO enabled, veth sends buffers
bigger than lower device's maximum GSO, forcing sw TSO and increasing
system CPU usage.

Signed-off-by: Solio Sarabia 
---
Exposing gso_max_size via sysfs is not advised [0]. This patch queries
available interfaces get this value. Reading dev_list is O(n), since it
can be large (e.g. hundreds of containers), only a subset of interfaces
is inspected.  _Please_ advise pointers how to make veth aware of lower
device's GSO value.

In a test scenario with Hyper-V, Ubuntu VM, Docker inside VM, and NTttcp
microworkload sending 40 Gbps from one container, this fix reduces 3x
sender host CPU overhead, since now all TSO is done on physical NIC.
Savings in CPU cycles benefit other use cases where veth is used, and
the GSO buffer size is properly set.

[0] https://lkml.org/lkml/2017/11/24/512

 drivers/net/veth.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index f5438d0..e255b51 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -298,6 +298,34 @@ static const struct net_device_ops veth_netdev_ops = {
   NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX | \
   NETIF_F_HW_VLAN_STAG_TX | NETIF_F_HW_VLAN_STAG_RX )
 
+static void veth_set_gso(struct net_device *dev)
+{
+   struct net_device *nd;
+   unsigned int size = GSO_MAX_SIZE;
+   u16 segs = GSO_MAX_SEGS;
+   unsigned int count = 0;
+   const unsigned int limit = 10;
+
+   /* Set default gso based on available physical/synthetic devices,
+* ignore virtual interfaces, and limit looping through dev_list
+* as the total number of interfaces can be large.
+*/
+   read_lock(_base_lock);
+   for_each_netdev(_net, nd) {
+   if (count >= limit)
+   break;
+   if (nd->dev.parent && nd->flags & IFF_UP) {
+   size = min(size, nd->gso_max_size);
+   segs = min(segs, nd->gso_max_segs);
+   }
+   count++;
+   }
+
+   read_unlock(_base_lock);
+   netif_set_gso_max_size(dev, size);
+   dev->gso_max_segs = size ? size - 1 : 0;
+}
+
 static void veth_setup(struct net_device *dev)
 {
ether_setup(dev);
@@ -323,6 +351,8 @@ static void veth_setup(struct net_device *dev)
dev->hw_features = VETH_FEATURES;
dev->hw_enc_features = VETH_FEATURES;
dev->mpls_features = NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
+
+   veth_set_gso(dev);
 }
 
 /*
-- 
2.7.4



Re: [PATCH] net: openvswitch: datapath: fix data type in queue_gso_packets

2017-11-25 Thread Willem de Bruijn
On Sat, Nov 25, 2017 at 2:14 PM, Gustavo A. R. Silva
 wrote:
> gso_type is being used in binary AND operations together with SKB_GSO_UDP.
> The issue is that variable gso_type is of type unsigned short and
> SKB_GSO_UDP expands to more than 16 bits:
>
> SKB_GSO_UDP = 1 << 16
>
> this makes any binary AND operation between gso_type and SKB_GSO_UDP to
> be always zero, hence making some code unreachable and likely causing
> undesired behavior.
>
> Fix this by changing the data type of variable gso_type to unsigned int.
>
> Addresses-Coverity-ID: 1462223
> Fixes: 0c19f846d582 ("net: accept UFO datagrams from tuntap and packet")
> Signed-off-by: Gustavo A. R. Silva 

Acked-by: Willem de Bruijn 

Good catch, thanks!

The issue here is that I brought back SKB_GSO_UDP at the end of the
list at 1 << 16 to avoid renaming of all that used to follow, while it used
to be defined as 1 << 1.

The skb_shinfo(skb)->gso_type field itself has been expanded as of v4.12
in commit 7f564528a480 ("skbuff: Extend gso_type to unsigned int.").

A quick scan shows a few nic drivers that also still cast to unsigned
short: bnxt, atl1c and qede. Since UFO hardware offload no longer
exists, this is safe wrt this patch. And it is fine for older kernels as
no driver supported the previous entry at 1 << 16, SKB_GSO_ESP.
But it is fragile wrt follow-on offloads. Probably a net-next patch.

The only other likely issue I spotted with the longer gso_type is
in trace events. net_dev_start_xmit and net_dev_rx_verbose_template
both export as u16.


[PATCH] net: openvswitch: datapath: fix data type in queue_gso_packets

2017-11-25 Thread Gustavo A. R. Silva
gso_type is being used in binary AND operations together with SKB_GSO_UDP.
The issue is that variable gso_type is of type unsigned short and
SKB_GSO_UDP expands to more than 16 bits:

SKB_GSO_UDP = 1 << 16

this makes any binary AND operation between gso_type and SKB_GSO_UDP to
be always zero, hence making some code unreachable and likely causing
undesired behavior.

Fix this by changing the data type of variable gso_type to unsigned int.

Addresses-Coverity-ID: 1462223
Fixes: 0c19f846d582 ("net: accept UFO datagrams from tuntap and packet")
Signed-off-by: Gustavo A. R. Silva 
---
 net/openvswitch/datapath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 99cfafc..ef38e5a 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -308,7 +308,7 @@ static int queue_gso_packets(struct datapath *dp, struct 
sk_buff *skb,
 const struct dp_upcall_info *upcall_info,
 uint32_t cutlen)
 {
-   unsigned short gso_type = skb_shinfo(skb)->gso_type;
+   unsigned int gso_type = skb_shinfo(skb)->gso_type;
struct sw_flow_key later_key;
struct sk_buff *segs, *nskb;
int err;
-- 
2.7.4



[PATCH V2] staging: rtl8188eu: Fix incorrect response to SIOCGIWESSID

2017-11-25 Thread Larry Finger
When not associated with an AP, wifi device drivers should respond to the
SIOCGIWESSID ioctl with a zero-length string for the SSID, which is the
behavior expected by dhcpcd.

Currently, this driver returns an error code (-1) from the ioctl call,
which causes dhcpcd to assume that the device is not a wireless interface
and therefore it fails to work correctly with it thereafter.

This problem was reported and tested at
https://github.com/lwfinger/rtl8188eu/issues/234.

Signed-off-by: Larry Finger 
---

v2 - completed missing subject
 drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index c0664dc80bf2..446310775e90 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -1395,19 +1395,13 @@ static int rtw_wx_get_essid(struct net_device *dev,
if ((check_fwstate(pmlmepriv, _FW_LINKED)) ||
(check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))) {
len = pcur_bss->Ssid.SsidLength;
-
-   wrqu->essid.length = len;
-
memcpy(extra, pcur_bss->Ssid.Ssid, len);
-
-   wrqu->essid.flags = 1;
} else {
-   ret = -1;
-   goto exit;
+   len = 0;
+   *extra = 0;
}
-
-exit:
-
+   wrqu->essid.length = len;
+   wrqu->essid.flags = 1;
 
return ret;
 }
-- 
2.13.6



Re: [PATCH] uapi: add SPDX identifier to vm_sockets_diag.h

2017-11-25 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 24 Nov 2017 12:08:40 -0800

> New file seems to have missed the SPDX license scan and update.
> 
> Signed-off-by: Stephen Hemminger 

Applied, thanks Stephen.


Re: [PATCH net] net: dsa: fix 'increment on 0' warning

2017-11-25 Thread David Miller
From: Vivien Didelot 
Date: Fri, 24 Nov 2017 11:36:06 -0500

> Setting the refcount to 0 when allocating a tree to match the number of
> switch devices it holds may cause an 'increment on 0; use-after-free',
> if CONFIG_REFCOUNT_FULL is enabled.
> 
> To fix this, do not decrement the refcount of a newly allocated tree,
> increment it when an already allocated tree is found, and decrement it
> after the probing of a switch, as done with the previous behavior.
> 
> At the same time, make dsa_tree_get and dsa_tree_put accept a NULL
> argument to simplify callers, and return the tree after incrementation,
> as most kref users like of_node_get and of_node_put do.
> 
> Fixes: 8e5bf9759a06 ("net: dsa: simplify tree reference counting")
> Signed-off-by: Vivien Didelot 

Applied, thanks Vivien.


Re: [PATCH v2] VSOCK: Don't call vsock_stream_has_data in atomic context

2017-11-25 Thread David Miller
From: Jorgen Hansen 
Date: Fri, 24 Nov 2017 06:25:28 -0800

> When using the host personality, VMCI will grab a mutex for any
> queue pair access. In the detach callback for the vmci vsock
> transport, we call vsock_stream_has_data while holding a spinlock,
> and vsock_stream_has_data will access a queue pair.
> 
> To avoid this, we can simply omit calling vsock_stream_has_data
> for host side queue pairs, since the QPs are empty per default
> when the guest has detached.
> 
> This bug affects users of VMware Workstation using kernel version
> 4.4 and later.
> 
> Testing: Ran vsock tests between guest and host, and verified that
> with this change, the host isn't calling vsock_stream_has_data
> during detach. Ran mixedTest between guest and host using both
> guest and host as server.
> 
> v2: Rebased on top of recent change to sk_state values
> Reviewed-by: Adit Ranadive 
> Reviewed-by: Aditya Sarwade 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Jorgen Hansen 

Applied, thank you.


[PATCH] staging: rtl8188eu:

2017-11-25 Thread Larry Finger
When not associated with an AP, wifi device drivers should respond to the
SIOCGIWESSID ioctl with a zero-length string for the SSID, which is the
behavior expected by dhcpcd.

Currently, this driver returns an error code (-1) from the ioctl call,
which causes dhcpcd to assume that the device is not a wireless interface
and therefore it fails to work correctly with it thereafter.

This problem was reported and tested at
https://github.com/lwfinger/rtl8188eu/issues/234.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index c0664dc80bf2..446310775e90 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -1395,19 +1395,13 @@ static int rtw_wx_get_essid(struct net_device *dev,
if ((check_fwstate(pmlmepriv, _FW_LINKED)) ||
(check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))) {
len = pcur_bss->Ssid.SsidLength;
-
-   wrqu->essid.length = len;
-
memcpy(extra, pcur_bss->Ssid.Ssid, len);
-
-   wrqu->essid.flags = 1;
} else {
-   ret = -1;
-   goto exit;
+   len = 0;
+   *extra = 0;
}
-
-exit:
-
+   wrqu->essid.length = len;
+   wrqu->essid.flags = 1;
 
return ret;
 }
-- 
2.13.6



Re: [PATCH] net: thunderbolt: Stop using zero to mean no valid DMA mapping

2017-11-25 Thread David Miller
From: Mika Westerberg 
Date: Fri, 24 Nov 2017 14:05:36 +0300

> Commit 86dabda426ac ("net: thunderbolt: Clear finished Tx frame bus
> address in tbnet_tx_callback()") fixed a DMA-API violation where the
> driver called dma_unmap_page() in tbnet_free_buffers() for a bus address
> that might already be unmapped. The fix was to zero out the bus address
> of a frame in tbnet_tx_callback().
> 
> However, as pointed out by David Miller, zero might well be valid
> mapping (at least in theory) so it is not good idea to use it here.
> 
> It turns out that we don't need the whole map/unmap dance for Tx buffers
> at all. Instead we can map the buffers when they are initially allocated
> and unmap them when the interface is brought down. In between we just
> DMA sync the buffers for the CPU or device as needed.
> 
> Signed-off-by: Mika Westerberg 

Applied, thanks.


Re: [PATCH net v3] net: thunderx: Fix TCP/UDP checksum offload for IPv6 pkts

2017-11-25 Thread David Miller
From: Aleksey Makarov 
Date: Thu, 23 Nov 2017 22:34:31 +0300

> From: Sunil Goutham 
> 
> Don't offload IP header checksum to NIC.
> 
> This fixes a previous patch which enabled checksum offloading
> for both IPv4 and IPv6 packets.  So L3 checksum offload was
> getting enabled for IPv6 pkts.  And HW is dropping these pkts
> as it assumes the pkt is IPv4 when IP csum offload is set
> in the SQ descriptor.
> 
> Fixes:  3a9024f52c2e ("net: thunderx: Enable TSO and checksum offloads for 
> ipv6")
> Signed-off-by: Sunil Goutham 
> Signed-off-by: Aleksey Makarov 
> Reviewed-by: Eric Dumazet 

Applied.


Re: [PATCH] dsa: dsa2: fix compile error for !CONFIG_OF

2017-11-25 Thread David Miller

Please repost this without the huge kernel config :-/


Re: [PATCH v4 2/2] sock: Move the socket inuse to namespace.

2017-11-25 Thread David Miller
From: Tonghao Zhang 
Date: Wed, 22 Nov 2017 17:51:25 -0800

> This patch add a member in struct netns_core. And this is
> a counter for socket_inuse in the _net_ namespace. The patch
> will add/sub counter in the sk_alloc or sk_free. Because socket and
> sock is in pair. It's a easy way to maintain the code and help
> developer to review. More important, it avoids holding the _net_
> namespace again.
> 
> Signed-off-by: Martin Zhang 
> Signed-off-by: Tonghao Zhang 

First, it is extremely unclear why this is better.  You do not explain
the reason at all.

Second:

> @@ -2646,17 +2646,8 @@ static int __init sock_init(void)
>  #ifdef CONFIG_PROC_FS
>  void socket_seq_show(struct seq_file *seq)
>  {
> - int cpu;
> - int counter = 0;
> -
> - for_each_possible_cpu(cpu)
> - counter += per_cpu(sockets_in_use, cpu);
> -
> - /* It can be negative, by the way. 8) */
> - if (counter < 0)
> - counter = 0;
> -
> - seq_printf(seq, "sockets: used %d\n", counter);

You've deleted the only use of "sockets_in_use" but you have not
removed that per-cpu variable and it's maintainence.

But do not even bother fixing this if you cannot explain properly
why these changes are an improvement.  I do not understant it,
and until I do I cannot consider these changes seriously.

Thank you.


[patch iproute2 00/11] tc: jsonify couple of qdics, filter and actions

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

An example json output:

$ tc -s -j filter show dev ens8 egress
[{
"protocol": "ip",
"pref": 6001,
"kind": "flower",
"chain": 0
},{
"protocol": "ip",
"pref": 6001,
"kind": "flower",
"chain": 0,
"options": {
"handle": 1,
"keys": {
"eth_type": "ipv4",
"dst_ip": "192.168.250.1"
},
"not_in_hw": true,
"actions": [{
"order": 1,
"kind": "gact",
"control_action": {
"type": "drop"
},
"prob": {
"random_type": "none",
"control_action": {
"type": "pass"
},
"val": 0
},
"index": 1,
"ref": 1,
"bind": 1,
"installed": 1667830,
"last_used": 1667830,
"stats": {
"bytes": 0,
"packets": 0,
"drops": 0,
"overlimits": 0,
"requeues": 0,
"backlog": 0,
"qlen": 0,
"requeues": 0
},
"cookie": "a1b2c3d4bb"
}
}
}
}
]
$ tc -s filter show dev ens8 egress
filter pref 6001 flower chain 0 
filter pref 6001 flower chain 0 handle 0x1 
  eth_type ipv4
  dst_ip 192.168.250.1
  not_in_hw
action order 1: gact action drop
 random type none pass val 0
 index 1 ref 1 bind 1 installed 16689 sec used 16689 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) 
backlog 0b 0p requeues 0
cookie a1b2c3d4bb

---
To be applied on top of my cookie fix patchset

Jiri Pirko (11):
  tc: jsonify qdisc core
  tc: jsonify stats2
  tc: jsonify fq_codel qdisc
  tc: jsonify htb qdisc
  tc: jsonify filter core
  tc: jsonify flower filter
  tc: jsonify matchall filter
  tc: jsonify actions core
  tc: jsonify gact action
  tc: jsonify mirred action
  tc: jsonify vlan action

 tc/f_flower.c   | 287 +---
 tc/f_matchall.c |  12 +--
 tc/m_action.c   |  22 +++--
 tc/m_gact.c |  18 ++--
 tc/m_mirred.c   |  46 +++--
 tc/m_vlan.c |  26 +++--
 tc/q_fq_codel.c |  25 +++--
 tc/q_htb.c  |  20 ++--
 tc/tc.c |   5 +-
 tc/tc_filter.c  |  47 ++
 tc/tc_qdisc.c   |  52 ++
 tc/tc_util.c|  66 +
 tc/tc_util.h|   1 +
 13 files changed, 396 insertions(+), 231 deletions(-)

-- 
2.9.5



[patch iproute2 06/11] tc: jsonify flower filter

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to flower filter.

Signed-off-by: Jiri Pirko 
---
 tc/f_flower.c | 287 ++
 1 file changed, 169 insertions(+), 118 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 9cad6c8..5a4ec83 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1037,89 +1037,105 @@ static int __mask_bits(char *addr, size_t len)
return bits;
 }
 
-static void flower_print_eth_addr(FILE *f, char *name,
- struct rtattr *addr_attr,
+static void flower_print_eth_addr(char *name, struct rtattr *addr_attr,
  struct rtattr *mask_attr)
 {
+   SPRINT_BUF(namefrm);
+   SPRINT_BUF(out);
SPRINT_BUF(b1);
+   size_t done;
int bits;
 
if (!addr_attr || RTA_PAYLOAD(addr_attr) != ETH_ALEN)
return;
-   fprintf(f, "\n  %s %s", name, ll_addr_n2a(RTA_DATA(addr_attr), ETH_ALEN,
- 0, b1, sizeof(b1)));
-   if (!mask_attr || RTA_PAYLOAD(mask_attr) != ETH_ALEN)
-   return;
-   bits = __mask_bits(RTA_DATA(mask_attr), ETH_ALEN);
-   if (bits < 0)
-   fprintf(f, "/%s", ll_addr_n2a(RTA_DATA(mask_attr), ETH_ALEN,
- 0, b1, sizeof(b1)));
-   else if (bits < ETH_ALEN * 8)
-   fprintf(f, "/%d", bits);
+   done = sprintf(out, "%s",
+  ll_addr_n2a(RTA_DATA(addr_attr), ETH_ALEN,
+  0, b1, sizeof(b1)));
+   if (mask_attr && RTA_PAYLOAD(mask_attr) == ETH_ALEN) {
+   bits = __mask_bits(RTA_DATA(mask_attr), ETH_ALEN);
+   if (bits < 0)
+   sprintf(out + done, "/%s",
+   ll_addr_n2a(RTA_DATA(mask_attr), ETH_ALEN,
+   0, b1, sizeof(b1)));
+   else if (bits < ETH_ALEN * 8)
+   sprintf(out + done, "/%d", bits);
+   }
+
+   sprintf(namefrm, "\n  %s %%s", name);
+   print_string(PRINT_ANY, name, namefrm, out);
 }
 
-static void flower_print_eth_type(FILE *f, __be16 *p_eth_type,
+static void flower_print_eth_type(__be16 *p_eth_type,
  struct rtattr *eth_type_attr)
 {
+   SPRINT_BUF(out);
__be16 eth_type;
 
if (!eth_type_attr)
return;
 
eth_type = rta_getattr_u16(eth_type_attr);
-   fprintf(f, "\n  eth_type ");
if (eth_type == htons(ETH_P_IP))
-   fprintf(f, "ipv4");
+   sprintf(out, "ipv4");
else if (eth_type == htons(ETH_P_IPV6))
-   fprintf(f, "ipv6");
+   sprintf(out, "ipv6");
else if (eth_type == htons(ETH_P_ARP))
-   fprintf(f, "arp");
+   sprintf(out, "arp");
else if (eth_type == htons(ETH_P_RARP))
-   fprintf(f, "rarp");
+   sprintf(out, "rarp");
else
-   fprintf(f, "%04x", ntohs(eth_type));
+   sprintf(out, "%04x", ntohs(eth_type));
+
+   print_string(PRINT_ANY, "eth_type", "\n  eth_type %s", out);
*p_eth_type = eth_type;
 }
 
-static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
+static void flower_print_ip_proto(__u8 *p_ip_proto,
  struct rtattr *ip_proto_attr)
 {
+   SPRINT_BUF(out);
__u8 ip_proto;
 
if (!ip_proto_attr)
return;
 
ip_proto = rta_getattr_u8(ip_proto_attr);
-   fprintf(f, "\n  ip_proto ");
if (ip_proto == IPPROTO_TCP)
-   fprintf(f, "tcp");
+   sprintf(out, "tcp");
else if (ip_proto == IPPROTO_UDP)
-   fprintf(f, "udp");
+   sprintf(out, "udp");
else if (ip_proto == IPPROTO_SCTP)
-   fprintf(f, "sctp");
+   sprintf(out, "sctp");
else if (ip_proto == IPPROTO_ICMP)
-   fprintf(f, "icmp");
+   sprintf(out, "icmp");
else if (ip_proto == IPPROTO_ICMPV6)
-   fprintf(f, "icmpv6");
+   sprintf(out, "icmpv6");
else
-   fprintf(f, "%02x", ip_proto);
+   sprintf(out, "%02x", ip_proto);
+
+   print_string(PRINT_ANY, "ip_proto", "\n  ip_proto %s", out);
*p_ip_proto = ip_proto;
 }
 
-static void flower_print_ip_attr(FILE *f, char *name,
-struct rtattr *key_attr,
+static void flower_print_ip_attr(char *name, struct rtattr *key_attr,
 struct rtattr *mask_attr)
 {
+   SPRINT_BUF(namefrm);
+   SPRINT_BUF(out);
+   size_t done;
+
if (!key_attr)
return;
 
-   fprintf(f, "\n  %s %x", name, rta_getattr_u8(key_attr));
-   if (!mask_attr)
-   return;
-   fprintf(f, "/%x", rta_getattr_u8(mask_attr));
+   

[patch iproute2 05/11] tc: jsonify filter core

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to filter core.

Signed-off-by: Jiri Pirko 
---
 tc/tc_filter.c | 47 ++-
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index 276a66c..3d9b889 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -237,33 +237,36 @@ int print_filter(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
return -1;
}
 
+   open_json_object(NULL);
+
if (n->nlmsg_type == RTM_DELTFILTER)
-   fprintf(fp, "deleted ");
+   print_bool(PRINT_ANY, "deleted", "deleted ", true);
 
if (n->nlmsg_type == RTM_NEWTFILTER &&
(n->nlmsg_flags & NLM_F_CREATE) &&
!(n->nlmsg_flags & NLM_F_EXCL))
-   fprintf(fp, "replaced ");
+   print_bool(PRINT_ANY, "replaced", "replaced ", true);
 
if (n->nlmsg_type == RTM_NEWTFILTER &&
(n->nlmsg_flags & NLM_F_CREATE) &&
(n->nlmsg_flags & NLM_F_EXCL))
-   fprintf(fp, "added ");
+   print_bool(PRINT_ANY, "added", "added ", true);
 
-   fprintf(fp, "filter ");
+   print_string(PRINT_FP, NULL, "filter ", NULL);
if (!filter_ifindex || filter_ifindex != t->tcm_ifindex)
-   fprintf(fp, "dev %s ", ll_index_to_name(t->tcm_ifindex));
+   print_string(PRINT_ANY, "dev", "dev %s ",
+ll_index_to_name(t->tcm_ifindex));
 
if (!filter_parent || filter_parent != t->tcm_parent) {
if (t->tcm_parent == TC_H_ROOT)
-   fprintf(fp, "root ");
+   print_bool(PRINT_ANY, "root", "root ", true);
else if (t->tcm_parent == TC_H_MAKE(TC_H_CLSACT, 
TC_H_MIN_INGRESS))
-   fprintf(fp, "ingress ");
+   print_bool(PRINT_ANY, "ingress", "ingress ", true);
else if (t->tcm_parent == TC_H_MAKE(TC_H_CLSACT, 
TC_H_MIN_EGRESS))
-   fprintf(fp, "egress ");
+   print_bool(PRINT_ANY, "egress", "egress ", true);
else {
print_tc_classid(abuf, sizeof(abuf), t->tcm_parent);
-   fprintf(fp, "parent %s ", abuf);
+   print_string(PRINT_ANY, "parent", "parent %s ", abuf);
}
}
 
@@ -274,39 +277,45 @@ int print_filter(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
if (!filter_protocol || filter_protocol != f_proto) {
if (f_proto) {
SPRINT_BUF(b1);
-   fprintf(fp, "protocol %s ",
-   ll_proto_n2a(f_proto, b1, sizeof(b1)));
+   print_string(PRINT_JSON, "protocol",
+"protocol %s ",
+ll_proto_n2a(f_proto, b1, 
sizeof(b1)));
}
}
if (!filter_prio || filter_prio != prio) {
if (prio)
-   fprintf(fp, "pref %u ", prio);
+   print_uint(PRINT_ANY, "pref", "pref %u ", prio);
}
}
-   fprintf(fp, "%s ", rta_getattr_str(tb[TCA_KIND]));
+   print_string(PRINT_ANY, "kind", "%s ", rta_getattr_str(tb[TCA_KIND]));
 
if (tb[TCA_CHAIN]) {
__u32 chain_index = rta_getattr_u32(tb[TCA_CHAIN]);
 
if (!filter_chain_index_set ||
filter_chain_index != chain_index)
-   fprintf(fp, "chain %u ", chain_index);
+   print_uint(PRINT_ANY, "chain", "chain %u ",
+  chain_index);
}
 
q = get_filter_kind(RTA_DATA(tb[TCA_KIND]));
if (tb[TCA_OPTIONS]) {
+   open_json_object("options");
if (q)
q->print_fopt(q, fp, tb[TCA_OPTIONS], t->tcm_handle);
else
-   fprintf(fp, "[cannot parse parameters]");
+   print_string(PRINT_FP, NULL,
+"[cannot parse parameters]", NULL);
+   close_json_object();
}
-   fprintf(fp, "\n");
+   print_string(PRINT_FP, NULL, "\n", NULL);
 
if (show_stats && (tb[TCA_STATS] || tb[TCA_STATS2])) {
print_tcstats_attr(fp, tb, " ", NULL);
-   fprintf(fp, "\n");
+   print_string(PRINT_FP, NULL, "\n", NULL);
}
 
+   close_json_object();
fflush(fp);
return 0;
 }
@@ -489,7 +498,9 @@ static int tc_filter_get(int cmd, unsigned int flags, int 
argc, char **argv)
return 2;
}
 
+   new_json_obj(json);

[patch iproute2 01/11] tc: jsonify qdisc core

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to qdisc core.

Signed-off-by: Jiri Pirko 
---
 tc/tc.c   |  5 -
 tc/tc_qdisc.c | 52 
 tc/tc_util.h  |  1 +
 3 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/tc/tc.c b/tc/tc.c
index 793dca9..ad9f07e 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -41,6 +41,7 @@ int batch_mode;
 int use_iec;
 int force;
 bool use_names;
+int json;
 
 static char *conf_file;
 
@@ -189,7 +190,7 @@ static void usage(void)
"   tc [-force] -batch filename\n"
"where  OBJECT := { qdisc | class | filter | action | 
monitor | exec }\n"
"   OPTIONS := { -s[tatistics] | -d[etails] | 
-r[aw] | -p[retty] | -b[atch] [filename] | -n[etns] name |\n"
-   "-nm | -nam[es] | { -cf | -conf } 
path }\n");
+   "-nm | -nam[es] | { -cf | -conf } 
path } | -j[son]\n");
 }
 
 static int do_cmd(int argc, char **argv)
@@ -312,6 +313,8 @@ int main(int argc, char **argv)
} else if (matches(argv[1], "-tshort") == 0) {
++timestamp;
++timestamp_short;
+   } else if (matches(argv[1], "-json") == 0) {
+   ++json;
} else {
fprintf(stderr, "Option \"%s\" is unknown, try \"tc 
-help\".\n", argv[1]);
return -1;
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index 2f88f11..4431d5f 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -227,39 +227,44 @@ int print_qdisc(const struct sockaddr_nl *who,
return -1;
}
 
+   open_json_object(NULL);
+
if (n->nlmsg_type == RTM_DELQDISC)
-   fprintf(fp, "deleted ");
+   print_bool(PRINT_ANY, "deleted", "deleted ", true);
 
if (n->nlmsg_type == RTM_NEWQDISC &&
(n->nlmsg_flags & NLM_F_CREATE) &&
(n->nlmsg_flags & NLM_F_REPLACE))
-   fprintf(fp, "replaced ");
+   print_bool(PRINT_ANY, "replaced", "replaced ", true);
 
if (n->nlmsg_type == RTM_NEWQDISC &&
(n->nlmsg_flags & NLM_F_CREATE) &&
(n->nlmsg_flags & NLM_F_EXCL))
-   fprintf(fp, "added ");
-
-   if (show_raw)
-   fprintf(fp, "qdisc %s %x:[%08x]  ",
-   rta_getattr_str(tb[TCA_KIND]),
-   t->tcm_handle >> 16, t->tcm_handle);
-   else
-   fprintf(fp, "qdisc %s %x: ", rta_getattr_str(tb[TCA_KIND]),
-   t->tcm_handle >> 16);
+   print_bool(PRINT_ANY, "added", "added ", true);
+
+   print_string(PRINT_ANY, "kind", "qdisc %s",
+rta_getattr_str(tb[TCA_KIND]));
+   sprintf(abuf, "%x:", t->tcm_handle >> 16);
+   print_string(PRINT_ANY, "handle", " %s", abuf);
+   if (show_raw) {
+   sprintf(abuf, "[%08x]", t->tcm_handle);
+   print_string(PRINT_FP, NULL, "%s", abuf);
+   }
+   print_string(PRINT_FP, NULL, " ", NULL);
 
if (filter_ifindex == 0)
-   fprintf(fp, "dev %s ", ll_index_to_name(t->tcm_ifindex));
+   print_string(PRINT_ANY, "dev", "dev %s ",
+ll_index_to_name(t->tcm_ifindex));
 
if (t->tcm_parent == TC_H_ROOT)
-   fprintf(fp, "root ");
+   print_bool(PRINT_ANY, "root", "root ", true);
else if (t->tcm_parent) {
print_tc_classid(abuf, sizeof(abuf), t->tcm_parent);
-   fprintf(fp, "parent %s ", abuf);
+   print_string(PRINT_ANY, "parent", "parent %s ", abuf);
}
 
if (t->tcm_info != 1)
-   fprintf(fp, "refcnt %d ", t->tcm_info);
+   print_uint(PRINT_ANY, "refcnt", "refcnt %u ", t->tcm_info);
 
/* pfifo_fast is generic enough to warrant the hardcoding --JHS */
if (strcmp("pfifo_fast", RTA_DATA(tb[TCA_KIND])) == 0)
@@ -267,17 +272,21 @@ int print_qdisc(const struct sockaddr_nl *who,
else
q = get_qdisc_kind(RTA_DATA(tb[TCA_KIND]));
 
+   open_json_object("options");
if (tb[TCA_OPTIONS]) {
if (q)
q->print_qopt(q, fp, tb[TCA_OPTIONS]);
else
-   fprintf(fp, "[cannot parse qdisc parameters]");
+   print_string(PRINT_FP, NULL,
+"[cannot parse qdisc parameters]", NULL);
}
-   fprintf(fp, "\n");
+   close_json_object();
+
+   print_string(PRINT_FP, NULL, "\n", NULL);
 
if (show_details && tb[TCA_STAB]) {
print_size_table(fp, " ", tb[TCA_STAB]);
-   fprintf(fp, "\n");
+   print_string(PRINT_FP, NULL, "\n", NULL);
}
 
if (show_stats) 

[patch iproute2 07/11] tc: jsonify matchall filter

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to matchall filter.

Signed-off-by: Jiri Pirko 
---
 tc/f_matchall.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tc/f_matchall.c b/tc/f_matchall.c
index 183f261..5ebd041 100644
--- a/tc/f_matchall.c
+++ b/tc/f_matchall.c
@@ -121,11 +121,11 @@ static int matchall_print_opt(struct filter_util *qu, 
FILE *f,
parse_rtattr_nested(tb, TCA_MATCHALL_MAX, opt);
 
if (handle)
-   fprintf(f, "handle 0x%x ", handle);
+   print_uint(PRINT_ANY, "handle", "handle 0x%x ", handle);
 
if (tb[TCA_MATCHALL_CLASSID]) {
SPRINT_BUF(b1);
-   fprintf(f, "flowid %s ",
+   print_string(PRINT_ANY, "flowid", "flowid %s ",

sprint_tc_classid(rta_getattr_u32(tb[TCA_MATCHALL_CLASSID]), b1));
}
 
@@ -133,14 +133,14 @@ static int matchall_print_opt(struct filter_util *qu, 
FILE *f,
__u32 flags = rta_getattr_u32(tb[TCA_MATCHALL_FLAGS]);
 
if (flags & TCA_CLS_FLAGS_SKIP_HW)
-   fprintf(f, "\n  skip_hw");
+   print_bool(PRINT_ANY, "skip_hw", "\n  skip_hw", true);
if (flags & TCA_CLS_FLAGS_SKIP_SW)
-   fprintf(f, "\n  skip_sw");
+   print_bool(PRINT_ANY, "skip_sw", "\n  skip_sw", true);
 
if (flags & TCA_CLS_FLAGS_IN_HW)
-   fprintf(f, "\n  in_hw");
+   print_bool(PRINT_ANY, "in_hw", "\n  in_hw", true);
else if (flags & TCA_CLS_FLAGS_NOT_IN_HW)
-   fprintf(f, "\n  not_in_hw");
+   print_bool(PRINT_ANY, "not_in_hw", "\n  not_in_hw", 
true);
}
 
if (tb[TCA_MATCHALL_ACT])
-- 
2.9.5



[patch iproute2 04/11] tc: jsonify htb qdisc

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to htb qdisc.

Signed-off-by: Jiri Pirko 
---
 tc/q_htb.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/tc/q_htb.c b/tc/q_htb.c
index ffb43aa..3fc2acb 100644
--- a/tc/q_htb.c
+++ b/tc/q_htb.c
@@ -293,9 +293,10 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, 
struct rtattr *opt)
if (RTA_PAYLOAD(tb[TCA_HTB_PARMS])  < sizeof(*hopt)) return -1;
 
if (!hopt->level) {
-   fprintf(f, "prio %d ", (int)hopt->prio);
+   print_int(PRINT_ANY, "prio", "prio ", (int)hopt->prio);
if (show_details)
-   fprintf(f, "quantum %d ", (int)hopt->quantum);
+   print_int(PRINT_ANY, "quantum", "quantum ",
+ (int)hopt->quantum);
}
 
rate64 = hopt->rate.rate;
@@ -341,16 +342,21 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, 
struct rtattr *opt)
gopt = RTA_DATA(tb[TCA_HTB_INIT]);
if (RTA_PAYLOAD(tb[TCA_HTB_INIT])  < sizeof(*gopt)) return -1;
 
-   fprintf(f, "r2q %d default %x direct_packets_stat %u",
-   gopt->rate2quantum, gopt->defcls, gopt->direct_pkts);
-   if (show_details)
-   fprintf(f, " ver %d.%d", gopt->version >> 16, 
gopt->version & 0x);
+   print_int(PRINT_ANY, "r2q", "r2q %d", gopt->rate2quantum);
+   print_uint(PRINT_ANY, "default", " default %u", gopt->defcls);
+   print_uint(PRINT_ANY, "direct_packets_stat",
+  " direct_packets_stat %u", gopt->direct_pkts);
+   if (show_details) {
+   sprintf(b1, "%d.%d", gopt->version >> 16, gopt->version 
& 0x);
+   print_string(PRINT_ANY, "ver", " ver %s", b1);
+   }
}
if (tb[TCA_HTB_DIRECT_QLEN] &&
RTA_PAYLOAD(tb[TCA_HTB_DIRECT_QLEN]) >= sizeof(__u32)) {
__u32 direct_qlen = rta_getattr_u32(tb[TCA_HTB_DIRECT_QLEN]);
 
-   fprintf(f, " direct_qlen %u", direct_qlen);
+   print_uint(PRINT_ANY, "direct_qlen", " direct_qlen %u",
+  direct_qlen);
}
return 0;
 }
-- 
2.9.5



[patch iproute2 10/11] tc: jsonify mirred action

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to mirred action.

Signed-off-by: Jiri Pirko 
---
 tc/m_mirred.c | 46 --
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/tc/m_mirred.c b/tc/m_mirred.c
index 33c915d..c771632 100644
--- a/tc/m_mirred.c
+++ b/tc/m_mirred.c
@@ -60,6 +60,34 @@ static const char *mirred_n2a(int action)
}
 }
 
+static const char *mirred_direction(int action)
+{
+   switch (action) {
+   case TCA_EGRESS_REDIR:
+   case TCA_EGRESS_MIRROR:
+   return "egress";
+   case TCA_INGRESS_REDIR:
+   case TCA_INGRESS_MIRROR:
+   return "ingress";
+   default:
+   return "unknown";
+   }
+}
+
+static const char *mirred_action(int action)
+{
+   switch (action) {
+   case TCA_EGRESS_REDIR:
+   case TCA_INGRESS_REDIR:
+   return "redirect";
+   case TCA_EGRESS_MIRROR:
+   case TCA_INGRESS_MIRROR:
+   return "mirror";
+   default:
+   return "unknown";
+   }
+}
+
 static int
 parse_direction(struct action_util *a, int *argc_p, char ***argv_p,
int tca_id, struct nlmsghdr *n)
@@ -254,7 +282,7 @@ print_mirred(struct action_util *au, FILE * f, struct 
rtattr *arg)
parse_rtattr_nested(tb, TCA_MIRRED_MAX, arg);
 
if (tb[TCA_MIRRED_PARMS] == NULL) {
-   fprintf(f, "[NULL mirred parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL mirred parameters]");
return -1;
}
p = RTA_DATA(tb[TCA_MIRRED_PARMS]);
@@ -269,12 +297,18 @@ print_mirred(struct action_util *au, FILE * f, struct 
rtattr *arg)
return -1;
}
 
-   fprintf(f, "mirred (%s to device %s)", mirred_n2a(p->eaction), dev);
+   print_string(PRINT_ANY, "kind", "%s ", "mirred");
+   print_string(PRINT_FP, NULL, "(%s", mirred_n2a(p->eaction));
+   print_string(PRINT_JSON, "mirred_action", NULL,
+mirred_action(p->eaction));
+   print_string(PRINT_JSON, "direction", NULL,
+mirred_direction(p->eaction));
+   print_string(PRINT_ANY, "to_dev", " to device %s)", dev);
print_action_control(f, " ", p->action, "");
 
-   fprintf(f, "\n ");
-   fprintf(f, "\tindex %u ref %d bind %d", p->index, p->refcnt,
-   p->bindcnt);
+   print_uint(PRINT_ANY, "index", "\n \tindex %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_MIRRED_TM]) {
@@ -283,7 +317,7 @@ print_mirred(struct action_util *au, FILE * f, struct 
rtattr *arg)
print_tm(f, tm);
}
}
-   fprintf(f, "\n ");
+   print_string(PRINT_FP, NULL, "%s", "\n ");
return 0;
 }
 
-- 
2.9.5



[patch iproute2 02/11] tc: jsonify stats2

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to stats2.

Signed-off-by: Jiri Pirko 
---
 tc/tc_util.c | 32 +++-
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 5532d40..d276987 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -781,16 +781,19 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, 
char *prefix, struct rtat
struct gnet_stats_basic bs = {0};
 
memcpy(, RTA_DATA(tbs[TCA_STATS_BASIC]), 
MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC]), sizeof(bs)));
-   fprintf(fp, "%sSent %llu bytes %u pkt",
-   prefix, (unsigned long long) bs.bytes, bs.packets);
+   print_string(PRINT_FP, NULL, "%s", prefix);
+   print_lluint(PRINT_ANY, "bytes", "Sent %llu bytes", bs.bytes);
+   print_uint(PRINT_ANY, "packets", " %u pkt", bs.packets);
}
 
if (tbs[TCA_STATS_QUEUE]) {
struct gnet_stats_queue q = {0};
 
memcpy(, RTA_DATA(tbs[TCA_STATS_QUEUE]), 
MIN(RTA_PAYLOAD(tbs[TCA_STATS_QUEUE]), sizeof(q)));
-   fprintf(fp, " (dropped %u, overlimits %u requeues %u) ",
-   q.drops, q.overlimits, q.requeues);
+   print_uint(PRINT_ANY, "drops", " (dropped %u", q.drops);
+   print_uint(PRINT_ANY, "overlimits", ", overlimits %u",
+  q.overlimits);
+   print_uint(PRINT_ANY, "requeues", " requeues %u) ", q.requeues);
}
 
if (tbs[TCA_STATS_RATE_EST64]) {
@@ -799,8 +802,11 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, 
char *prefix, struct rtat
memcpy(, RTA_DATA(tbs[TCA_STATS_RATE_EST64]),
   MIN(RTA_PAYLOAD(tbs[TCA_STATS_RATE_EST64]),
   sizeof(re)));
-   fprintf(fp, "\n%srate %s %llupps ",
-   prefix, sprint_rate(re.bps, b1), re.pps);
+   print_string(PRINT_FP, NULL, "\n%s", prefix);
+   print_lluint(PRINT_JSON, "rate", NULL, re.bps);
+   print_string(PRINT_FP, NULL, "rate %s",
+sprint_rate(re.bps, b1));
+   print_lluint(PRINT_ANY, "pps", " %llupps", re.pps);
} else if (tbs[TCA_STATS_RATE_EST]) {
struct gnet_stats_rate_est re = {0};
 
@@ -808,6 +814,11 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, 
char *prefix, struct rtat
   MIN(RTA_PAYLOAD(tbs[TCA_STATS_RATE_EST]), sizeof(re)));
fprintf(fp, "\n%srate %s %upps ",
prefix, sprint_rate(re.bps, b1), re.pps);
+   print_string(PRINT_FP, NULL, "\n%s", prefix);
+   print_uint(PRINT_JSON, "rate", NULL, re.bps);
+   print_string(PRINT_FP, NULL, "rate %s",
+sprint_rate(re.bps, b1));
+   print_uint(PRINT_ANY, "pps", " %upps", re.pps);
}
 
if (tbs[TCA_STATS_QUEUE]) {
@@ -815,9 +826,12 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, 
char *prefix, struct rtat
 
memcpy(, RTA_DATA(tbs[TCA_STATS_QUEUE]), 
MIN(RTA_PAYLOAD(tbs[TCA_STATS_QUEUE]), sizeof(q)));
if (!tbs[TCA_STATS_RATE_EST])
-   fprintf(fp, "\n%s", prefix);
-   fprintf(fp, "backlog %s %up requeues %u ",
-   sprint_size(q.backlog, b1), q.qlen, q.requeues);
+   print_string(PRINT_FP, NULL, "\n%s", prefix);
+   print_uint(PRINT_JSON, "backlog", NULL, q.backlog);
+   print_string(PRINT_FP, NULL, "backlog %s",
+sprint_size(q.backlog, b1));
+   print_uint(PRINT_ANY, "qlen", " %up", q.qlen);
+   print_uint(PRINT_ANY, "requeues", " requeues %u", q.qlen);
}
 
if (xstats)
-- 
2.9.5



[patch iproute2 08/11] tc: jsonify actions core

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to actions core.

Signed-off-by: Jiri Pirko 
---
 tc/m_action.c | 22 +++---
 tc/tc_util.c  | 34 --
 2 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index e6d9149..0940a39 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -301,17 +301,19 @@ static int tc_print_one_action(FILE *f, struct rtattr 
*arg)
return err;
 
if (show_stats && tb[TCA_ACT_STATS]) {
-   fprintf(f, "\tAction statistics:\n");
+   print_string(PRINT_FP, NULL, "\tAction statistics:\n", NULL);
+   open_json_object("stats");
print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
-   fprintf(f, "\n");
+   close_json_object();
+   print_string(PRINT_FP, NULL, "\n", NULL);
}
if (tb[TCA_ACT_COOKIE]) {
int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
char b1[strsz * 2 + 1];
 
-   fprintf(f, "\tcookie %s\n",
-   hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
- strsz, b1, sizeof(b1)));
+   print_string(PRINT_ANY, "cookie", "\tcookie %s\n",
+hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
+  strsz, b1, sizeof(b1)));
}
 
return 0;
@@ -362,15 +364,21 @@ 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_array(PRINT_JSON, "actions");
for (i = 0; i < tot_acts; i++) {
if (tb[i]) {
-   fprintf(f, "\n\taction order %d: ", i);
+   open_json_object(NULL);
+   print_uint(PRINT_ANY, "order",
+  "\n\taction order %u: ", i);
if (tc_print_one_action(f, tb[i]) < 0) {
-   fprintf(f, "Error printing action\n");
+   print_string(PRINT_FP, NULL,
+"Error printing action\n", NULL);
}
+   close_json_object();
}
 
}
+   close_json_object();
 
return 0;
 }
diff --git a/tc/tc_util.c b/tc/tc_util.c
index d276987..1887905 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -709,12 +709,17 @@ int parse_action_control_slash(int *argc_p, char 
***argv_p,
 void print_action_control(FILE *f, const char *prefix,
  int action, const char *suffix)
 {
-   fprintf(f, "%s%s", prefix, action_n2a(action));
+   print_string(PRINT_FP, NULL, "%s", prefix);
+   open_json_object("control_action");
+   print_string(PRINT_ANY, "type", "%s", action_n2a(action));
if (TC_ACT_EXT_CMP(action, TC_ACT_GOTO_CHAIN))
-   fprintf(f, " chain %u", action & TC_ACT_EXT_VAL_MASK);
+   print_uint(PRINT_ANY, "chain", " chain %u",
+  action & TC_ACT_EXT_VAL_MASK);
if (TC_ACT_EXT_CMP(action, TC_ACT_JUMP))
-   fprintf(f, " %u", action & TC_ACT_EXT_VAL_MASK);
-   fprintf(f, "%s", suffix);
+   print_uint(PRINT_ANY, "jump", " %u",
+  action & TC_ACT_EXT_VAL_MASK);
+   close_json_object();
+   print_string(PRINT_FP, NULL, "%s", suffix);
 }
 
 int get_linklayer(unsigned int *val, const char *arg)
@@ -762,12 +767,21 @@ void print_tm(FILE *f, const struct tcf_t *tm)
 {
int hz = get_user_hz();
 
-   if (tm->install != 0)
-   fprintf(f, " installed %u sec", (unsigned int)(tm->install/hz));
-   if (tm->lastuse != 0)
-   fprintf(f, " used %u sec", (unsigned int)(tm->lastuse/hz));
-   if (tm->expires != 0)
-   fprintf(f, " expires %u sec", (unsigned int)(tm->expires/hz));
+   if (tm->install != 0) {
+   print_uint(PRINT_JSON, "installed", NULL, tm->install);
+   print_uint(PRINT_FP, NULL, " installed %u sec",
+  (unsigned int)(tm->install/hz));
+   }
+   if (tm->lastuse != 0) {
+   print_uint(PRINT_JSON, "last_used", NULL, tm->lastuse);
+   print_uint(PRINT_FP, NULL, " used %u sec",
+  (unsigned int)(tm->lastuse/hz));
+   }
+   if (tm->expires != 0) {
+   print_uint(PRINT_JSON, "expires", NULL, tm->expires);
+   print_uint(PRINT_FP, NULL, " expires %u sec",
+  (unsigned int)(tm->expires/hz));
+   }
 }
 
 void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct 
rtattr **xstats)
-- 
2.9.5



[patch iproute2 03/11] tc: jsonify fq_codel qdisc

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to fq_codel qdisc.

Signed-off-by: Jiri Pirko 
---
 tc/q_fq_codel.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/tc/q_fq_codel.c b/tc/q_fq_codel.c
index 86c6fb2..fd1f59c 100644
--- a/tc/q_fq_codel.c
+++ b/tc/q_fq_codel.c
@@ -173,44 +173,51 @@ static int fq_codel_print_opt(struct qdisc_util *qu, FILE 
*f, struct rtattr *opt
if (tb[TCA_FQ_CODEL_LIMIT] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_LIMIT]) >= sizeof(__u32)) {
limit = rta_getattr_u32(tb[TCA_FQ_CODEL_LIMIT]);
-   fprintf(f, "limit %up ", limit);
+   print_uint(PRINT_ANY, "limit", "limit %up ", limit);
}
if (tb[TCA_FQ_CODEL_FLOWS] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_FLOWS]) >= sizeof(__u32)) {
flows = rta_getattr_u32(tb[TCA_FQ_CODEL_FLOWS]);
-   fprintf(f, "flows %u ", flows);
+   print_uint(PRINT_ANY, "flows", "flows %u ", flows);
}
if (tb[TCA_FQ_CODEL_QUANTUM] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_QUANTUM]) >= sizeof(__u32)) {
quantum = rta_getattr_u32(tb[TCA_FQ_CODEL_QUANTUM]);
-   fprintf(f, "quantum %u ", quantum);
+   print_uint(PRINT_ANY, "quantum", "quantum %u ", quantum);
}
if (tb[TCA_FQ_CODEL_TARGET] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_TARGET]) >= sizeof(__u32)) {
target = rta_getattr_u32(tb[TCA_FQ_CODEL_TARGET]);
-   fprintf(f, "target %s ", sprint_time(target, b1));
+   print_uint(PRINT_JSON, "target", NULL, target);
+   print_string(PRINT_FP, NULL, "target %s ",
+sprint_time(target, b1));
}
if (tb[TCA_FQ_CODEL_CE_THRESHOLD] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_CE_THRESHOLD]) >= sizeof(__u32)) {
ce_threshold = rta_getattr_u32(tb[TCA_FQ_CODEL_CE_THRESHOLD]);
-   fprintf(f, "ce_threshold %s ", sprint_time(ce_threshold, b1));
+   print_uint(PRINT_JSON, "ce_threshold", NULL, ce_threshold);
+   print_string(PRINT_FP, NULL, "ce_threshold %s ",
+sprint_time(ce_threshold, b1));
}
if (tb[TCA_FQ_CODEL_INTERVAL] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_INTERVAL]) >= sizeof(__u32)) {
interval = rta_getattr_u32(tb[TCA_FQ_CODEL_INTERVAL]);
-   fprintf(f, "interval %s ", sprint_time(interval, b1));
+   print_uint(PRINT_JSON, "interval", NULL, interval);
+   print_string(PRINT_FP, NULL, "interval %s ",
+sprint_time(interval, b1));
}
if (tb[TCA_FQ_CODEL_MEMORY_LIMIT] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_MEMORY_LIMIT]) >= sizeof(__u32)) {
memory_limit = rta_getattr_u32(tb[TCA_FQ_CODEL_MEMORY_LIMIT]);
-
-   fprintf(f, "memory_limit %s ", sprint_size(memory_limit, b1));
+   print_uint(PRINT_JSON, "memory_limit", NULL, memory_limit);
+   print_string(PRINT_FP, NULL, "memory_limit %s ",
+sprint_size(memory_limit, b1));
}
if (tb[TCA_FQ_CODEL_ECN] &&
RTA_PAYLOAD(tb[TCA_FQ_CODEL_ECN]) >= sizeof(__u32)) {
ecn = rta_getattr_u32(tb[TCA_FQ_CODEL_ECN]);
if (ecn)
-   fprintf(f, "ecn ");
+   print_bool(PRINT_ANY, "ecn", "ecn ", true);
}
 
return 0;
-- 
2.9.5



[patch iproute2 09/11] tc: jsonify gact action

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to gact action.

Signed-off-by: Jiri Pirko 
---
 tc/m_gact.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tc/m_gact.c b/tc/m_gact.c
index efe992f..905aa51 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -175,12 +175,12 @@ print_gact(struct action_util *au, FILE * f, struct 
rtattr *arg)
parse_rtattr_nested(tb, TCA_GACT_MAX, arg);
 
if (tb[TCA_GACT_PARMS] == NULL) {
-   fprintf(f, "[NULL gact parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL gact parameters]");
return -1;
}
p = RTA_DATA(tb[TCA_GACT_PARMS]);
 
-   fprintf(f, "gact ");
+   print_string(PRINT_ANY, "kind", "%s ", "gact");
print_action_control(f, "action ", p->action, "");
 #ifdef CONFIG_GACT_PROB
if (tb[TCA_GACT_PROB] != NULL) {
@@ -190,12 +190,16 @@ print_gact(struct action_util *au, FILE * f, struct 
rtattr *arg)
memset(_dummy, 0, sizeof(pp_dummy));
pp = _dummy;
}
-   fprintf(f, "\n\t random type %s", prob_n2a(pp->ptype));
+   open_json_object("prob");
+   print_string(PRINT_ANY, "random_type", "\n\t random type %s",
+prob_n2a(pp->ptype));
print_action_control(f, " ", pp->paction, " ");
-   fprintf(f, "val %d", pp->pval);
+   print_int(PRINT_ANY, "val", "val %d", pp->pval);
+   close_json_object();
 #endif
-   fprintf(f, "\n\t index %u ref %d bind %d", p->index, p->refcnt,
-   p->bindcnt);
+   print_uint(PRINT_ANY, "index", "\n\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_GACT_TM]) {
struct tcf_t *tm = RTA_DATA(tb[TCA_GACT_TM]);
@@ -203,7 +207,7 @@ print_gact(struct action_util *au, FILE * f, struct rtattr 
*arg)
print_tm(f, tm);
}
}
-   fprintf(f, "\n ");
+   print_string(PRINT_FP, NULL, "%s", "\n");
return 0;
 }
 
-- 
2.9.5



[patch iproute2 11/11] tc: jsonify vlan action

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Add json output to vlan action.

Signed-off-by: Jiri Pirko 
---
 tc/m_vlan.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index cccb499..44254b6 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -195,39 +195,37 @@ static int print_vlan(struct action_util *au, FILE *f, 
struct rtattr *arg)
parse_rtattr_nested(tb, TCA_VLAN_MAX, arg);
 
if (!tb[TCA_VLAN_PARMS]) {
-   fprintf(f, "[NULL vlan parameters]");
+   print_string(PRINT_FP, NULL, "%s", "[NULL vlan parameters]");
return -1;
}
parm = RTA_DATA(tb[TCA_VLAN_PARMS]);
 
-   fprintf(f, " vlan");
+   print_string(PRINT_ANY, "kind", "%s ", "vlan");
+   print_string(PRINT_ANY, "vlan_action", " %s", 
action_names[parm->v_action]);
 
switch (parm->v_action) {
-   case TCA_VLAN_ACT_POP:
-   fprintf(f, " pop");
-   break;
case TCA_VLAN_ACT_PUSH:
case TCA_VLAN_ACT_MODIFY:
-   fprintf(f, " %s", action_names[parm->v_action]);
if (tb[TCA_VLAN_PUSH_VLAN_ID]) {
val = rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
-   fprintf(f, " id %u", val);
+   print_uint(PRINT_ANY, "id", " id %u", val);
}
if (tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]) {
-   fprintf(f, " protocol %s",
-   
ll_proto_n2a(rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]),
-b1, sizeof(b1)));
+   print_string(PRINT_ANY, "protocol", " protocol %s",
+
ll_proto_n2a(rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]),
+ b1, sizeof(b1)));
}
if (tb[TCA_VLAN_PUSH_VLAN_PRIORITY]) {
val = rta_getattr_u8(tb[TCA_VLAN_PUSH_VLAN_PRIORITY]);
-   fprintf(f, " priority %u", val);
+   print_uint(PRINT_ANY, "priority", " priority %u", val);
}
break;
}
print_action_control(f, " ", parm->action, "");
 
-   fprintf(f, "\n\t index %u ref %d bind %d", parm->index, parm->refcnt,
-   parm->bindcnt);
+   print_uint(PRINT_ANY, "index", "\n\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_VLAN_TM]) {
@@ -237,7 +235,7 @@ static int print_vlan(struct action_util *au, FILE *f, 
struct rtattr *arg)
}
}
 
-   fprintf(f, "\n ");
+   print_string(PRINT_FP, NULL, "%s", "\n");
 
return 0;
 }
-- 
2.9.5



[PATCH net] openvswitch: fix the incorrect flow action alloc size

2017-11-25 Thread zhangliping
From: zhangliping 

If we want to add a datapath flow, which has more than 500 vxlan outputs'
action, we will get the following error reports:
  openvswitch: netlink: Flow action size 32832 bytes exceeds max
  openvswitch: netlink: Flow action size 32832 bytes exceeds max
  openvswitch: netlink: Actions may not be safe on all matching packets
  ... ...

It seems that we can simply enlarge the MAX_ACTIONS_BUFSIZE to fix it, but
this is not the root cause. For example, for a vxlan output action, we need
about 60 bytes for the nlattr, but after it is converted to the flow
action, it only occupies 24 bytes. This means that we can still support
more than 1000 vxlan output actions for a single datapath flow under the
the current 32k max limitation.

So even if the nla_len(attr) is larger than MAX_ACTIONS_BUFSIZE, we
shouldn't report EINVAL and keep it move on, as the judgement can be
done by the reserve_sfa_size.

Signed-off-by: zhangliping 
---
 net/openvswitch/flow_netlink.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index dc42479..624ea74 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2241,14 +2241,11 @@ int ovs_nla_put_mask(const struct sw_flow *flow, struct 
sk_buff *skb)
 
 #define MAX_ACTIONS_BUFSIZE(32 * 1024)
 
-static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log)
+static struct sw_flow_actions *nla_alloc_flow_actions(int size)
 {
struct sw_flow_actions *sfa;
 
-   if (size > MAX_ACTIONS_BUFSIZE) {
-   OVS_NLERR(log, "Flow action size %u bytes exceeds max", size);
-   return ERR_PTR(-EINVAL);
-   }
+   WARN_ON_ONCE(size > MAX_ACTIONS_BUFSIZE);
 
sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL);
if (!sfa)
@@ -2321,12 +2318,15 @@ static struct nlattr *reserve_sfa_size(struct 
sw_flow_actions **sfa,
new_acts_size = ksize(*sfa) * 2;
 
if (new_acts_size > MAX_ACTIONS_BUFSIZE) {
-   if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size)
+   if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size) {
+   OVS_NLERR(log, "Flow action size exceeds max %u",
+ MAX_ACTIONS_BUFSIZE);
return ERR_PTR(-EMSGSIZE);
+   }
new_acts_size = MAX_ACTIONS_BUFSIZE;
}
 
-   acts = nla_alloc_flow_actions(new_acts_size, log);
+   acts = nla_alloc_flow_actions(new_acts_size);
if (IS_ERR(acts))
return (void *)acts;
 
@@ -3059,7 +3059,7 @@ int ovs_nla_copy_actions(struct net *net, const struct 
nlattr *attr,
 {
int err;
 
-   *sfa = nla_alloc_flow_actions(nla_len(attr), log);
+   *sfa = nla_alloc_flow_actions(min(nla_len(attr), MAX_ACTIONS_BUFSIZE));
if (IS_ERR(*sfa))
return PTR_ERR(*sfa);
 
-- 
2.5.5




[PATCH net 3/3] sctp: do not abandon the other frags in unsent outq if one msg has outstanding frags

2017-11-25 Thread Xin Long
Now for the abandoned chunks in unsent outq, it would just free the chunks.
Because no tsn is assigned to them yet, there's no need to send fwd tsn to
peer, unlike for the abandoned chunks in sent outq.

The problem is when parts of the msg have been sent and the other frags
are still in unsent outq, if they are abandoned/dropped, the peer would
never get this msg reassembled.

So these frags in unsent outq can't be dropped if this msg already has
outstanding frags.

This patch does the check in sctp_chunk_abandoned and
sctp_prsctp_prune_unsent.

Signed-off-by: Xin Long 
---
 net/sctp/chunk.c| 4 
 net/sctp/outqueue.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 9213805..7f8baa4 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -308,6 +308,10 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
if (chunk->msg->abandoned)
return 1;
 
+   if (!chunk->has_tsn &&
+   !(chunk->chunk_hdr->flags & SCTP_DATA_FIRST_FRAG))
+   return 0;
+
if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
time_after(jiffies, chunk->msg->expires_at)) {
struct sctp_stream_out *streamout =
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 4ab164b..7d67fee 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -407,7 +407,8 @@ static int sctp_prsctp_prune_unsent(struct sctp_association 
*asoc,
 
list_for_each_entry_safe(chk, temp, >out_chunk_list, list) {
if (!chk->msg->abandoned &&
-   (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
+   (!(chk->chunk_hdr->flags & SCTP_DATA_FIRST_FRAG) ||
+!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
 chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive))
continue;
 
-- 
2.1.0



[PATCH net 2/3] sctp: abandon the whole msg if one part of a fragmented message is abandoned

2017-11-25 Thread Xin Long
As rfc3758#section-3.1 demands:

   A3) When a TSN is "abandoned", if it is part of a fragmented message,
   all other TSN's within that fragmented message MUST be abandoned
   at the same time.

Besides, if it couldn't handle this, the rest frags would never get
assembled in peer side.

This patch supports it by adding abandoned flag in sctp_datamsg, when
one chunk is being abandoned, set chunk->msg->abandoned as well. Next
time when checking for abandoned, go checking chunk->msg->abandoned
first.

Signed-off-by: Xin Long 
---
 include/net/sctp/structs.h |  3 ++-
 net/sctp/chunk.c   |  7 +++
 net/sctp/outqueue.c| 12 
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 16f949e..2f8f93d 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -503,7 +503,8 @@ struct sctp_datamsg {
/* Did the messenge fail to send? */
int send_error;
u8 send_failed:1,
-  can_delay;   /* should this message be Nagle delayed */
+  can_delay:1, /* should this message be Nagle delayed */
+  abandoned:1; /* should this message be abandoned */
 };
 
 struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *,
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 7b261af..9213805 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -53,6 +53,7 @@ static void sctp_datamsg_init(struct sctp_datamsg *msg)
msg->send_failed = 0;
msg->send_error = 0;
msg->can_delay = 1;
+   msg->abandoned = 0;
msg->expires_at = 0;
INIT_LIST_HEAD(>chunks);
 }
@@ -304,6 +305,9 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
if (!chunk->asoc->peer.prsctp_capable)
return 0;
 
+   if (chunk->msg->abandoned)
+   return 1;
+
if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
time_after(jiffies, chunk->msg->expires_at)) {
struct sctp_stream_out *streamout =
@@ -316,6 +320,7 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
chunk->asoc->abandoned_unsent[SCTP_PR_INDEX(TTL)]++;
streamout->ext->abandoned_unsent[SCTP_PR_INDEX(TTL)]++;
}
+   chunk->msg->abandoned = 1;
return 1;
} else if (SCTP_PR_RTX_ENABLED(chunk->sinfo.sinfo_flags) &&
   chunk->sent_count > chunk->sinfo.sinfo_timetolive) {
@@ -324,10 +329,12 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 
chunk->asoc->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
streamout->ext->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
+   chunk->msg->abandoned = 1;
return 1;
} else if (!SCTP_PR_POLICY(chunk->sinfo.sinfo_flags) &&
   chunk->msg->expires_at &&
   time_after(jiffies, chunk->msg->expires_at)) {
+   chunk->msg->abandoned = 1;
return 1;
}
/* PRIO policy is processed by sendmsg, not here */
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 7029f8b..4ab164b 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -364,10 +364,12 @@ static int sctp_prsctp_prune_sent(struct sctp_association 
*asoc,
list_for_each_entry_safe(chk, temp, queue, transmitted_list) {
struct sctp_stream_out *streamout;
 
-   if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
-   chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
+   if (!chk->msg->abandoned &&
+   (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
+chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive))
continue;
 
+   chk->msg->abandoned = 1;
list_del_init(>transmitted_list);
sctp_insert_list(>outqueue.abandoned,
 >transmitted_list);
@@ -404,10 +406,12 @@ static int sctp_prsctp_prune_unsent(struct 
sctp_association *asoc,
q->sched->unsched_all(>stream);
 
list_for_each_entry_safe(chk, temp, >out_chunk_list, list) {
-   if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
-   chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
+   if (!chk->msg->abandoned &&
+   (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
+chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive))
continue;
 
+   chk->msg->abandoned = 1;
sctp_sched_dequeue_common(q, chk);
asoc->sent_cnt_removable--;
asoc->abandoned_unsent[SCTP_PR_INDEX(PRIO)]++;
-- 
2.1.0



[PATCH net 1/3] sctp: only update outstanding_bytes for transmitted queue when doing prsctp_prune

2017-11-25 Thread Xin Long
Now outstanding_bytes is only increased when appending chunks into one
packet and sending it at 1st time, while decreased when it is about to
move into retransmit queue. It means outstanding_bytes value is already
decreased for all chunks in retransmit queue.

However sctp_prsctp_prune_sent is a common function to check the chunks
in both transmitted and retransmit queue, it decrease outstanding_bytes
when moving a chunk into abandoned queue from either of them.

It could cause outstanding_bytes underflow, as it also decreases it's
value for the chunks in retransmit queue.

This patch fixes it by only updating outstanding_bytes for transmitted
queue when pruning queues for prsctp prio policy, the same fix is also
needed in sctp_check_transmitted.

Fixes: 8dbdf1f5b09c ("sctp: implement prsctp PRIO policy")
Signed-off-by: Xin Long 
---
 net/sctp/outqueue.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 4db012a..7029f8b 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -377,7 +377,8 @@ static int sctp_prsctp_prune_sent(struct sctp_association 
*asoc,
asoc->abandoned_sent[SCTP_PR_INDEX(PRIO)]++;
streamout->ext->abandoned_sent[SCTP_PR_INDEX(PRIO)]++;
 
-   if (!chk->tsn_gap_acked) {
+   if (queue != >outqueue.retransmit &&
+   !chk->tsn_gap_acked) {
if (chk->transport)
chk->transport->flight_size -=
sctp_data_size(chk);
@@ -1434,7 +1435,8 @@ static void sctp_check_transmitted(struct sctp_outq *q,
/* If this chunk has not been acked, stop
 * considering it as 'outstanding'.
 */
-   if (!tchunk->tsn_gap_acked) {
+   if (transmitted_queue != >retransmit &&
+   !tchunk->tsn_gap_acked) {
if (tchunk->transport)
tchunk->transport->flight_size -=
sctp_data_size(tchunk);
-- 
2.1.0



[PATCH net 0/3] a couple of fixes for chunks abandoned in prsctp

2017-11-25 Thread Xin Long
Now when abandoning chunks in prsctp, it doesn't consider for frags in
one msg, which would cause peer can never receive the whole frags for
one msg to get them reassembled, these pieces of this msg will stay in
the reasm queue forever and block the following chunks' receiving.

This patchset is to fix them in patch 2 and 3, and also fix another
issue for prsctp in patch 1.

Xin Long (3):
  sctp: only update outstanding_bytes for transmitted queue when doing
prsctp_prune
  sctp: abandon the whole msg if one part of a fragmented message is
abandoned
  sctp: do not abandon the other frags in unsent outq if one msg has
outstanding frags

 include/net/sctp/structs.h |  3 ++-
 net/sctp/chunk.c   | 11 +++
 net/sctp/outqueue.c| 19 +--
 3 files changed, 26 insertions(+), 7 deletions(-)

-- 
2.1.0



[PATCH net 4/5] sctp: avoid flushing unsent queue when doing asoc reset

2017-11-25 Thread Xin Long
Now when doing asoc reset, it cleans up sacked and abandoned queues
by calling sctp_outq_free where it also cleans up unsent, retransmit
and transmitted queues.

It's safe for the sender of response, as these 3 queues are empty at
that time. But when the receiver of response is doing the reset, the
users may already enqueue some chunks into unsent during the time
waiting the response, and these chunks should not be flushed.

To void the chunks in it would be removed, it moves the queue into a
temp list, then gets it back after sctp_outq_free is done.

The patch also fixes some incorrect comments in
sctp_process_strreset_tsnreq.

Signed-off-by: Xin Long 
---
 net/sctp/stream.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index f3b7d27..9dd5bfe 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -747,9 +747,10 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
goto out;
}
 
-   /* G3: The same processing as though a SACK chunk with no gap report
-* and a cumulative TSN ACK of the Sender's Next TSN minus 1 were
-* received MUST be performed.
+   /* G4: The same processing as though a FWD-TSN chunk (as defined in
+* [RFC3758]) with all streams affected and a new cumulative TSN
+* ACK of the Receiver's Next TSN minus 1 were received MUST be
+* performed.
 */
max_tsn_seen = sctp_tsnmap_get_max_tsn_seen(>peer.tsn_map);
sctp_ulpq_reasm_flushtsn(>ulpq, max_tsn_seen);
@@ -764,10 +765,9 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
sctp_tsnmap_init(>peer.tsn_map, SCTP_TSN_MAP_INITIAL,
 init_tsn, GFP_ATOMIC);
 
-   /* G4: The same processing as though a FWD-TSN chunk (as defined in
-* [RFC3758]) with all streams affected and a new cumulative TSN
-* ACK of the Receiver's Next TSN minus 1 were received MUST be
-* performed.
+   /* G3: The same processing as though a SACK chunk with no gap report
+* and a cumulative TSN ACK of the Sender's Next TSN minus 1 were
+* received MUST be performed.
 */
sctp_outq_free(>outqueue);
 
@@ -1021,6 +1021,7 @@ struct sctp_chunk *sctp_process_strreset_resp(
if (result == SCTP_STRRESET_PERFORMED) {
__u32 mtsn = sctp_tsnmap_get_max_tsn_seen(
>peer.tsn_map);
+   LIST_HEAD(temp);
 
sctp_ulpq_reasm_flushtsn(>ulpq, mtsn);
sctp_ulpq_abort_pd(>ulpq, GFP_ATOMIC);
@@ -1029,7 +1030,13 @@ struct sctp_chunk *sctp_process_strreset_resp(
 SCTP_TSN_MAP_INITIAL,
 stsn, GFP_ATOMIC);
 
+   /* Clean up sacked and abandoned queues only. As the
+* out_chunk_list may not be empty, splice it to temp,
+* then get it back after sctp_outq_free is done.
+*/
+   list_splice_init(>outqueue.out_chunk_list, );
sctp_outq_free(>outqueue);
+   list_splice_init(, >outqueue.out_chunk_list);
 
asoc->next_tsn = rtsn;
asoc->ctsn_ack_point = asoc->next_tsn - 1;
-- 
2.1.0



[PATCH net 5/5] sctp: set sender next_tsn for the old result with ctsn_ack_point plus 1

2017-11-25 Thread Xin Long
When doing asoc reset, if the sender of the response has already sent some
chunk and increased asoc->next_tsn before the duplicate request comes, the
response will use the old result with an incorrect sender next_tsn.

Better than asoc->next_tsn, asoc->ctsn_ack_point can't be changed after
the sender of the response has performed the asoc reset and before the
peer has confirmed it, and it's value is still asoc->next_tsn original
value minus 1.

This patch sets sender next_tsn for the old result with ctsn_ack_point
plus 1 when processing the duplicate request, to make sure the sender
next_tsn value peer gets will be always right.

Fixes: 692787cef651 ("sctp: implement receiver-side procedures for the SSN/TSN 
Reset Request Parameter")
Signed-off-by: Xin Long 
---
 net/sctp/stream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 9dd5bfe..a20145b 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -725,7 +725,7 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
i = asoc->strreset_inseq - request_seq - 1;
result = asoc->strreset_result[i];
if (result == SCTP_STRRESET_PERFORMED) {
-   next_tsn = asoc->next_tsn;
+   next_tsn = asoc->ctsn_ack_point + 1;
init_tsn =
sctp_tsnmap_get_ctsn(>peer.tsn_map) + 1;
}
-- 
2.1.0



[PATCH net 3/5] sctp: only allow the asoc reset when the asoc outq is empty

2017-11-25 Thread Xin Long
As it says in rfc6525#section5.1.4, before sending the request,

   C2:  The sender has either no outstanding TSNs or considers all
outstanding TSNs abandoned.

Prior to this patch, it tried to consider all outstanding TSNs abandoned
by dropping all chunks in all outqs with sctp_outq_free (even including
sacked, retransmit and transmitted queues) when doing this reset, which
is too aggressive.

To make it work gently, this patch will only allow the asoc reset when
the sender has no outstanding TSNs by checking if unsent, transmitted
and retransmit are all empty with sctp_outq_is_empty before sending
and processing the request.

Fixes: 692787cef651 ("sctp: implement receiver-side procedures for the SSN/TSN 
Reset Request Parameter")
Signed-off-by: Xin Long 
---
 net/sctp/stream.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index b209037..f3b7d27 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -406,6 +406,9 @@ int sctp_send_reset_assoc(struct sctp_association *asoc)
if (asoc->strreset_outstanding)
return -EINPROGRESS;
 
+   if (!sctp_outq_is_empty(>outqueue))
+   return -EAGAIN;
+
chunk = sctp_make_strreset_tsnreq(asoc);
if (!chunk)
return -ENOMEM;
@@ -728,6 +731,12 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
}
goto err;
}
+
+   if (!sctp_outq_is_empty(>outqueue)) {
+   result = SCTP_STRRESET_IN_PROGRESS;
+   goto err;
+   }
+
asoc->strreset_inseq++;
 
if (!(asoc->strreset_enable & SCTP_ENABLE_RESET_ASSOC_REQ))
-- 
2.1.0



[PATCH net 2/5] sctp: only allow the out stream reset when the stream outq is empty

2017-11-25 Thread Xin Long
Now the out stream reset in sctp stream reconf could be done even if
the stream outq is not empty. It means that users can not be sure
since which msg the new ssn will be used.

To make this more synchronous, it shouldn't allow to do out stream
reset until these chunks in unsent outq all are sent out.

This patch checks the corresponding stream outqs when sending and
processing the request . If any of them has unsent chunks in outq,
it will return -EAGAIN instead or send SCTP_STRRESET_IN_PROGRESS
back to the sender.

Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset 
Request Parameter")
Suggested-by: Marcelo Ricardo Leitner 
Signed-off-by: Xin Long 
---
 net/sctp/stream.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 09c797a..b209037 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -254,6 +254,30 @@ static int sctp_send_reconf(struct sctp_association *asoc,
return retval;
 }
 
+static bool sctp_stream_outq_is_empty(struct sctp_stream *stream,
+ __u16 str_nums, __be16 *str_list)
+{
+   struct sctp_association *asoc;
+   __u16 i;
+
+   asoc = container_of(stream, struct sctp_association, stream);
+   if (!asoc->outqueue.out_qlen)
+   return true;
+
+   if (!str_nums)
+   return false;
+
+   for (i = 0; i < str_nums; i++) {
+   __u16 sid = ntohs(str_list[i]);
+
+   if (stream->out[sid].ext &&
+   !list_empty(>out[sid].ext->outq))
+   return false;
+   }
+
+   return true;
+}
+
 int sctp_send_reset_streams(struct sctp_association *asoc,
struct sctp_reset_streams *params)
 {
@@ -317,6 +341,11 @@ int sctp_send_reset_streams(struct sctp_association *asoc,
for (i = 0; i < str_nums; i++)
nstr_list[i] = htons(str_list[i]);
 
+   if (out && !sctp_stream_outq_is_empty(stream, str_nums, nstr_list)) {
+   retval = -EAGAIN;
+   goto out;
+   }
+
chunk = sctp_make_strreset_req(asoc, str_nums, nstr_list, out, in);
 
kfree(nstr_list);
@@ -636,6 +665,12 @@ struct sctp_chunk *sctp_process_strreset_inreq(
}
}
 
+   if (!sctp_stream_outq_is_empty(stream, nums, str_p)) {
+   result = SCTP_STRRESET_IN_PROGRESS;
+   asoc->strreset_inseq--;
+   goto err;
+   }
+
chunk = sctp_make_strreset_req(asoc, nums, str_p, 1, 0);
if (!chunk)
goto out;
-- 
2.1.0



[PATCH net 0/5] sctp: a bunch of fixes for stream reconfig

2017-11-25 Thread Xin Long
This patchset is to make stream reset and asoc reset work more correctly
for stream reconfig.

Thank to Marcelo making them very clear.

Xin Long (5):
  sctp: use sizeof(__u16) for each stream number length instead of magic
number
  sctp: only allow the out stream reset when the stream outq is empty
  sctp: only allow the asoc reset when the asoc outq is empty
  sctp: avoid flushing unsent queue when doing asoc reset
  sctp: set sender next_tsn for the old result with ctsn_ack_point plus
1

 net/sctp/stream.c | 77 ++-
 1 file changed, 65 insertions(+), 12 deletions(-)

-- 
2.1.0



[PATCH net 1/5] sctp: use sizeof(__u16) for each stream number length instead of magic number

2017-11-25 Thread Xin Long
Now in stream reconf part there are still some places using magic
number 2 for each stream number length. To make it more readable,
this patch is to replace them with sizeof(__u16).

Reported-by: Marcelo Ricardo Leitner 
Signed-off-by: Xin Long 
---
 net/sctp/stream.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index a11db21..09c797a 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -563,7 +563,7 @@ struct sctp_chunk *sctp_process_strreset_outreq(
flags = SCTP_STREAM_RESET_INCOMING_SSN;
}
 
-   nums = (ntohs(param.p->length) - sizeof(*outreq)) / 2;
+   nums = (ntohs(param.p->length) - sizeof(*outreq)) / sizeof(__u16);
if (nums) {
str_p = outreq->list_of_streams;
for (i = 0; i < nums; i++) {
@@ -627,7 +627,7 @@ struct sctp_chunk *sctp_process_strreset_inreq(
goto out;
}
 
-   nums = (ntohs(param.p->length) - sizeof(*inreq)) / 2;
+   nums = (ntohs(param.p->length) - sizeof(*inreq)) / sizeof(__u16);
str_p = inreq->list_of_streams;
for (i = 0; i < nums; i++) {
if (ntohs(str_p[i]) >= stream->outcnt) {
@@ -927,7 +927,8 @@ struct sctp_chunk *sctp_process_strreset_resp(
 
outreq = (struct sctp_strreset_outreq *)req;
str_p = outreq->list_of_streams;
-   nums = (ntohs(outreq->param_hdr.length) - sizeof(*outreq)) / 2;
+   nums = (ntohs(outreq->param_hdr.length) - sizeof(*outreq)) /
+  sizeof(__u16);
 
if (result == SCTP_STRRESET_PERFORMED) {
if (nums) {
@@ -956,7 +957,8 @@ struct sctp_chunk *sctp_process_strreset_resp(
 
inreq = (struct sctp_strreset_inreq *)req;
str_p = inreq->list_of_streams;
-   nums = (ntohs(inreq->param_hdr.length) - sizeof(*inreq)) / 2;
+   nums = (ntohs(inreq->param_hdr.length) - sizeof(*inreq)) /
+  sizeof(__u16);
 
*evp = sctp_ulpevent_make_stream_reset_event(asoc, flags,
nums, str_p, GFP_ATOMIC);
-- 
2.1.0



[PATCH] net: ethernet: davinci_emac: Deduplicate bus_find_device() by name matching

2017-11-25 Thread Lukas Wunner
No need to reinvent the wheel, we have bus_find_device_by_name().

Cc: Grygorii Strashko 
Signed-off-by: Lukas Wunner 
---
 drivers/net/ethernet/ti/davinci_emac.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c 
b/drivers/net/ethernet/ti/davinci_emac.c
index 4bb561856af5..f58c0c620356 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1385,11 +1385,6 @@ static int emac_devioctl(struct net_device *ndev, struct 
ifreq *ifrq, int cmd)
return -EOPNOTSUPP;
 }
 
-static int match_first_device(struct device *dev, void *data)
-{
-   return !strncmp(dev_name(dev), "davinci_mdio", 12);
-}
-
 /**
  * emac_dev_open - EMAC device open
  * @ndev: The DaVinci EMAC network adapter
@@ -1489,8 +1484,8 @@ static int emac_dev_open(struct net_device *ndev)
 
/* use the first phy on the bus if pdata did not give us a phy id */
if (!phydev && !priv->phy_id) {
-   phy = bus_find_device(_bus_type, NULL, NULL,
- match_first_device);
+   phy = bus_find_device_by_name(_bus_type, NULL,
+ "davinci_mdio");
if (phy) {
priv->phy_id = dev_name(phy);
if (!priv->phy_id || !*priv->phy_id)
-- 
2.11.0



[patch iproute2 0/2] tc: couple ouf action cookie printout fixes

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Jiri Pirko (2):
  tc: move action cookie print out of the stats if
  tc: remove action cookie len from printout

 tc/m_action.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
2.9.5



[patch iproute2 2/2] tc: remove action cookie len from printout

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Make the output same as input and avoid printout of unnecessary len.

Suggested-by: Stephen Hemminger 
Fixes: fd8b3d2c1b9b ("actions: Add support for user cookies")
Signed-off-by: Jiri Pirko 
---
 tc/m_action.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index aa3aa52..e6d9149 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -309,7 +309,7 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg)
int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
char b1[strsz * 2 + 1];
 
-   fprintf(f, "\tcookie len %d %s\n", strsz,
+   fprintf(f, "\tcookie %s\n",
hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
  strsz, b1, sizeof(b1)));
}
-- 
2.9.5



[patch iproute2 1/2] tc: move action cookie print out of the stats if

2017-11-25 Thread Jiri Pirko
From: Jiri Pirko 

Cookie print was made dependent on show_stats for no good reason. Fix
this bu pushing cookie print ot of the stats if.

Fixes: fd8b3d2c1b9b ("actions: Add support for user cookies")
Signed-off-by: Jiri Pirko 
---
 tc/m_action.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 85f9e27..aa3aa52 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -301,19 +301,18 @@ static int tc_print_one_action(FILE *f, struct rtattr 
*arg)
return err;
 
if (show_stats && tb[TCA_ACT_STATS]) {
-
fprintf(f, "\tAction statistics:\n");
print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
-   if (tb[TCA_ACT_COOKIE]) {
-   int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
-   char b1[strsz * 2 + 1];
-
-   fprintf(f, "\n\tcookie len %d %s ", strsz,
-   hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
- strsz, b1, sizeof(b1)));
-   }
fprintf(f, "\n");
}
+   if (tb[TCA_ACT_COOKIE]) {
+   int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
+   char b1[strsz * 2 + 1];
+
+   fprintf(f, "\tcookie len %d %s\n", strsz,
+   hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
+ strsz, b1, sizeof(b1)));
+   }
 
return 0;
 }
-- 
2.9.5



Re: [patch iproute2] tc: move action cookie print out of the stats if

2017-11-25 Thread Jiri Pirko
Fri, Nov 24, 2017 at 06:11:42PM CET, step...@networkplumber.org wrote:
>On Fri, 24 Nov 2017 09:28:21 +0100
>Jiri Pirko  wrote:
>
>> From: Jiri Pirko 
>> 
>> Cookie print was made dependent on show_stats for no good reason. Fix
>> this bu pushing cookie print ot of the stats if.
>> 
>> Fixes: fd8b3d2c1b9b ("actions: Add support for user cookies")
>> Signed-off-by: Jiri Pirko 
>> ---
>>  tc/m_action.c | 17 -
>>  1 file changed, 8 insertions(+), 9 deletions(-)
>> 
>> diff --git a/tc/m_action.c b/tc/m_action.c
>> index 0dce97f..c2fc4f1 100644
>> --- a/tc/m_action.c
>> +++ b/tc/m_action.c
>> @@ -301,19 +301,18 @@ static int tc_print_one_action(FILE *f, struct rtattr 
>> *arg)
>>  return err;
>>  
>>  if (show_stats && tb[TCA_ACT_STATS]) {
>> -
>>  fprintf(f, "\tAction statistics:\n");
>>  print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
>> -if (tb[TCA_ACT_COOKIE]) {
>> -int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
>> -char b1[strsz * 2 + 1];
>> -
>> -fprintf(f, "\n\tcookie len %d %s ", strsz,
>> -hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
>> -  strsz, b1, sizeof(b1)));
>> -}
>>  fprintf(f, "\n");
>>  }
>> +if (tb[TCA_ACT_COOKIE]) {
>> +int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
>> +char b1[strsz * 2 + 1];
>> +
>> +fprintf(f, "\tcookie len %d %s\n", strsz,
>> +hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
>> +  strsz, b1, sizeof(b1)));
>> +}
>>  
>>  return 0;
>>  }
>
>Yes, it should not be under stats flag.
>The general model is that -s is for statistics only; and -d is for read only 
>detail values.
>So this makes sense.
>
>The problem is that the format of the action cookie needs to be same on 
>command line
>argument and on display; i.e drop the length part of the display .

I agree. But that should be a separate patch.