From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [patch iproute2 v6 1/3] lib/libnetlink: Add a function rtnl_talk_msg Date: Fri, 5 Jan 2018 10:50:50 -0700 Message-ID: <6562fc22-4161-5efc-488a-79a62db1cd4f@gmail.com> References: <20180104073454.11867-1-chrism@mellanox.com> <20180104073454.11867-2-chrism@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: gerlitz.or@gmail.com, stephen@networkplumber.org, marcelo.leitner@gmail.com To: Chris Mi , netdev@vger.kernel.org Return-path: Received: from mail-pf0-f194.google.com ([209.85.192.194]:44640 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752013AbeAERux (ORCPT ); Fri, 5 Jan 2018 12:50:53 -0500 Received: by mail-pf0-f194.google.com with SMTP id m26so2452477pfj.11 for ; Fri, 05 Jan 2018 09:50:53 -0800 (PST) In-Reply-To: <20180104073454.11867-2-chrism@mellanox.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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 > --- > 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.