Re: [dpdk-dev] [PATCH v2 1/4] lib: introduce IF Proxy library

["Andrzej Ostruszka [C]" <aostruszka@marvell.com>]

First of all, please excuse me Stephen for late reply.

On 02/07/2020 02:34, Stephen Hemminger wrote:
> I had great hopes for this library, because such code exists in
> almost every real world application. But the current version falls
> short.

Sorry to disappoint.

> So what this library does is turn a message based protocol (netlink)
> into a set of callbacks. Not sure if that is all that useful processing
> messages is often easier.

Callbacks or queued notifications.  I'm not sure I understand why you 
are saying that it would be easier for application to do everything on 
its own when it can now just pass a callback or dequeue notification 
with all essential information extracted.  Could you elaborate here?

> It would be more useful if the library took the netlink and maintained
> a set of tables (interfaces, neighbours, routes) using DPDK infrastructure
> and did it as efficiently as possible with RCU.

I'm pretty sure it would, but that's a bit like complaining about new 
toy that it doesn't have batteries included.  And BTW "no batteries" was 
a conscientious decision.  I wanted something basic that would ease 
sniffing of the network configuration and IMHO it does that.

I don't have access to environment where heavy netlink processing would 
be required.  And apart from that I can imagine that every application 
has its own particular needs so it will be difficult to please 
everybody.  So instead I thought it would be better to come up with a 
starting point upon which this thing can grow - with input from the 
community/users.

> On a real world router with 1M routes, the processing of netlink can be
> a significant chore. It has to be done efficiently. Probably needs to use
> rte_ctrl_thread() and not overload the interrupt thread.

That is another voice against using interrupt thread.  I acknowledge 
your (plural) concerns and will think about changing it.  Thanks.  I'm 
not sure if I manage to make it before .08 though.

> Also, please don't reinvent netlink parsing. Use a standard library
> like libmnl.

I actually checked this and decided that this will not buy me much and 
will add an additional dependency.  I also checked how other DPDK parts 
(there are couple) and iproute2 utils are handling netlink and decided 
to follow their style of manual handling.  Could you give a hint here 
how hard you want to push for libmnl?  As I've mentioned, that was on my 
list of options, so if community does not mind adding libmnl dependency 
and would prefer using it instead of manual parsing then I might adapt 
to that.

> Please use doxygen format for API documentation
> 
>> +/* This function should be called by the implementation whenever it notices
>> + * change in the network configuration.  The arguments are:
>> + * - ev : pointer to filled event data structure (all fields are expected to be
>> + *     filled, with the exception of 'port_id' for all proxy/port related
>> + *     events: this function clones the event notification for each bound port
>> + *     and fills 'port_id' appropriately).
>> + * - px : proxy node when given event is proxy/port related, otherwise pass NULL
>> + */
>> +void ifpx_notify_event(struct rte_ifpx_event *ev, struct ifpx_proxy_node *px);
>> +

Will do - thank you.

Thank you for taking time to look at this.

With regards
Andrzej Ostruszka