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]

Message-ID: <20171227134024.0706d33f@xeon-e3>
Date:   Wed, 27 Dec 2017 13:40:24 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
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, 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.