From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner Subject: Re: [patch iproute2 v3 3/4] tc: Add -bs option to batch mode Date: Wed, 27 Dec 2017 20:06:00 -0200 Message-ID: <20171227220600.GC22042@localhost.localdomain> References: <20171225084658.24076-1-chrism@mellanox.com> <20171225084658.24076-4-chrism@mellanox.com> <28563dd5-01be-9198-2911-658bbd0ba3d7@gmail.com> <20171227203929.GB22042@localhost.localdomain> <20171227134024.0706d33f@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: David Ahern , Chris Mi , netdev@vger.kernel.org, gerlitz.or@gmail.com To: Stephen Hemminger Return-path: Received: from mx1.redhat.com ([209.132.183.28]:51708 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751871AbdL0WGD (ORCPT ); Wed, 27 Dec 2017 17:06:03 -0500 Content-Disposition: inline In-Reply-To: <20171227134024.0706d33f@xeon-e3> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Dec 27, 2017 at 01:40:24PM -0800, Stephen Hemminger wrote: > On Wed, 27 Dec 2017 18:39:29 -0200 > Marcelo Ricardo Leitner wrote: > > > > > + send = false; > > > > + else > > > > + send = true; > > > > + > > > > + ret = do_cmd(largc, largv, batch_size, msg_iov_index++, send); > > > > > > What happens if tc commands are interlaced in the file -- qdisc add, > > > class add, filter add, then a delete, show, exec, etc.? Right now each > > > command is handled one at a time so an add followed by a delete will > > > work. Your proposed batching loop won't work for this case as some > > > commands are executed when that line is reached and others are batched > > > for later send. Not all of the tc commands need to be batched in a > > > single message so perhaps those commands cause the queue to be flushed > > > (ie, message sent), then that command is executed and you start the > > > batching over. > > > > > > Further, I really think the batching can be done without the global > > > variables and without the command handlers knowing it is batching or > > > part of an iov. e.g., in the case of batching try having the commands > > > malloc the request buffer and return the pointer back to this loop in > > > which case this loop calls rtnl_talk_msg and frees the buffers. > > > > Sounds like the batching is being done at the wrong level. If it was > > done by rtnl_talk(), it should be easier. > > We can keep rtnl_talk() for previous users and make rtnl_talk_msg() do > > the batching, mostly independent of which kind of msg it it. > > > > As you need to inform it that it was the last entry, that may be > > detected with feof(stdin). Just add a 'bool flush' parameter to it. > > rtnl_talk_msg(...., flush=feof(stdin)); > > > > Next step then would be to add a memory manager layer to it, so > > libnetlink wouldn't need to copy the messages but recycle pointers: > > rtnl_get_msgbuf(): returns a buffer that one can use to fill in the > > msg and use with rtnl_talk_msg() > > and the free is done by libnetlink itself when the message is > > finally sent, so no need to keep track of what one needs to free or > > can reuse. > > > What about using sendmmsg instead? > That woudl allow sending multiple messages in one syscall. Could be. Although the batching effect would be very different. sendmmsg calls cond_resched() between messages, for instance.