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

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]

Date:   Wed, 27 Dec 2017 20:06:00 -0200
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     David Ahern <dsahern@...il.com>, Chris Mi <chrism@...lanox.com>,
        netdev@...r.kernel.org, gerlitz.or@...il.com
Subject: Re: [patch iproute2 v3 3/4] tc: Add -bs option to batch mode

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@...il.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.