netdev - Re: [patch iproute2 v6 1/3] lib/libnetlink: Add a function rtnl_talk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6562fc22-4161-5efc-488a-79a62db1cd4f@gmail.com>
Date:   Fri, 5 Jan 2018 10:50:50 -0700
From:   David Ahern <dsahern@...il.com>
To:     Chris Mi <chrism@...lanox.com>, netdev@...r.kernel.org
Cc:     gerlitz.or@...il.com, stephen@...workplumber.org,
        marcelo.leitner@...il.com
Subject: Re: [patch iproute2 v6 1/3] lib/libnetlink: Add a function
 rtnl_talk_msg

On 1/4/18 12:34 AM, Chris Mi wrote:
> rtnl_talk can only send a single message to kernel. Add a new function
> rtnl_talk_msg that can send multiple messages to kernel.
> 
> Signed-off-by: Chris Mi <chrism@...lanox.com>
> ---
>  include/libnetlink.h |  3 +++
>  lib/libnetlink.c     | 66 ++++++++++++++++++++++++++++++++++++++--------------
>  2 files changed, 51 insertions(+), 18 deletions(-)
> 

I think you should add an argument to rtnl_talk_msg to return the number
of messages processed. That can be used to refine which line failed. As
batch size increases the current design puts the burden on the user to
scan a lot of lines to find the one that fails:

tc -b tc.batch  -bs 50
RTNETLINK answers: File exists
We have an error talking to the kernel, -1
Command failed tc.batch:2-51

We should be able to tell them exactly which line failed.

Also, it would be better to call this rtnl_talk_iov, take an iov as an
argument and have a common rtnl_talk_msg for existing code and this new one.

As it stands you are having to add:
   struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };

to tc functions when it really only needs to know about iov's.