> On Thu, Feb 24, 2022 at 02:46:24PM +0100, Thomas Monjalon wrote: > > 24/02/2022 12:06, Ananyev, Konstantin: > > > > > > > > > > > >> Or have a generic library for reading LPM entries. > > > > > > > > > > > >> L3fwd is supposed > > > > > > > > > > > >> to be as small as possible (it no longer is), and the > > > > > > > > > > > >> real work should > > > > > > > > > > > >> be done by libraries to make it easier to build other > > > > > > > > > > > >> applications. > > > > > > > > > > > > > > > > > > > > > > > > I never heard users ask about such thing, > > > > > > > > > > > > but if there is a demand for that, then I suppose it > > > > > > > > > > > > could be considered. > > > > > > > > > > > > CC-ing LPM/FIB maintainers to comment. > > > > > > > > > > > > Though I believe it should be a subject of separate > > > > > > > > > > > > patch and discussion > > > > > > > > > > > > (I think many questions will arise - what format should > > > > > > > > > > > > be, how to support > > > > > > > > > > > > different types of user-data, to make it generic > > > > > > > > > > > > enough, etc.). > > > > > > > > > > > > > > > > > > > > > > Agree, it is very application specific, so it could be > > > > > > > > > > > really difficult > > > > > > > > > > > to make it generic. > > > > > > > > > > > > > > > > > > > > But several other also have LPM tables, so why not have > > > > > > > > > > common code for other applications. > > > > > > > > > > > > > > > > > > > > examples/l3fwd-power/main.c > > > > > > > > > > examples/ipsec-secgw/rt.c > > > > > > > > > > examples/ip_fragmentation/main.c > > > > > > > > > > examples/l3fwd/l3fwd_lpm.c > > > > > > > > > > examples/ip_reassembly/main.c > > > > > > > > > > > > > > > > > > Ah yes, that's good point. > > > > > > > > > All these examples (except ipsec-secgw) started as l3fwd > > > > > > > > > clones, > > > > > > > > > so all of them have hard-coded LPM (and EM) tables too. > > > > > > > > > Yes it would be good thing to address that problem too, > > > > > > > > > and have some common code (and common routes file format) for > > > > > > > > > all of them. > > > > > > > > > I don't know is that a good idea to introduce parse file > > > > > > > > > function in LPM/FIB library > > > > > > > > > itself, might be better to have something like > > > > > > > > > examples/common/lpm_parse*. > > > > > > > > > Anyway, this is an extra effort, and I think no-one has time > > > > > > > > > for it in 22.03 timeframe. > > > > > > > > > My suggestion would be for 22.03 go ahead with current l3fwd > > > > > > > > > patches, > > > > > > > > > then later we can consider to make it common and update other > > > > > > > > > examples. > > > > > > > > > > > > > > > > I don't think this patch is urgent. > > > > > > > > I suggest taking time to have common code for all examples > > > > > > > > and target a merge in DPDK 22.07. > > > > > > > > > > > > > > Well, yes, from one perspective it not really a critical one, > > > > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year > > > > > > > by now. > > > > > > > Though l3fwd is one of mostly used examples inside DPDK and > > > > > > > it is quite a pain to patch/rebuild it each time someone needs to > > > > > > > run > > > > > > > l3fwd with a different routing table. > > > > > > > Merging this patch will allow people to use l3fwd for more > > > > > > > realistic test > > > > > > > scenarios in a painless manner. > > > > > > > So I believe this patch is really helpful and should be > > > > > > > beneficial for the whole community. > > > > > > > Looking from that perspective, I don't see why it has to be "all > > > > > > > or nothing" attitude here. > > > > > > > Why we can't move one step at a time instead? > > > > > > > That would allow to split and effort in terms of > > > > > > > development/testing/upstreaming/etc. > > > > > > > > > > > > When a feature is merged, there is less incentives to rework. > > > > > > That's why, when a feature is not urgent, > > > > > > it is better to wait for the complete work. > > > > > > > > > > That's true till some extent, though from other side > > > > > even without further rework that patch improves situation > > > > > from what we have right now. > > > > > So I don't see any harm here. > > > > > > > > It is adding a lot of code to an example which is already too big. > > > > There are a lot of complain about the size of l3fwd. > > > > That's why I think it makes sense to require this extra code > > > > (not demonstrating anything, but just for testing convenience) > > > > outside of the example. > > > > > > Ok, so your main concern is l3fwd code size increase, right? > > > Then would it help if for we'll move file parsing code into a separate > > > file(s) > > > (under examples/l3fwd) for now? > > > Something like examples/l3fwd/(lpm_em)_route_parse.c. > > > > Yes it would help to isolate the config file parsing code. > > What others think? > > > I still would like config code for loading an LPM table or FIB table to be > put inside the relevant libraries themselves, rather than having it in the > examples themselves (even if shared between them).
Honestly, I don't see any good reasons for that: I presume users of these libraries already have their own routing config files with their own format requirements and I suppose these formats differ a lot (depending on use-case). For DPDK internal purposes (provide sample apps with ability to load routes dynamically) some common code inside examples/ seems more than enough. Konstantin
