Re: [patch iproute2 v3 3/4] tc: Add -bs option to batch mode

[Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>]

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 <marcelo.leitner@gmail.com> 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.