[dpdk-dev] difficulty w/ RTE_NEXT_ABI

[Matthew Hall]

On Sat, Nov 21, 2015 at 11:44:20AM +0100, Thomas Monjalon wrote:
> The new mbuf provides packet type instead of flags.
> So the processing in this function is changed and the variable name is
> different to reflect this.

But the data type of the variable is the same, and this is an internal 
always_inline function.

So again I am confused what advantage we got from RTE_NEXT_ABI here, and how 
you have multiple copies of RTE_NEXT_ABI on a single symbol when it is a 
binary variable.

This doesn't really answer the bigger question about the reasoning.

Matthew.

[dpdk-dev] difficulty w/ RTE_NEXT_ABI

[Thomas Monjalon]

2015-11-21 03:49, Matthew Hall:
> I was trying to rebase my DPDK onto v2.1.0 and I came across some very 
> confusing code in examples/l3fwd/main.c .
> 
> So... this code used the RTE_NEXT_ABI macros on a change which does not 
> appear 
> to affect the API... on a function that is marked always_inline ???
> 
> Maybe I missed something but this seems pointless. An always_inline function 
> is going to have to be recompiled in any case.
> 
> Now I have no clue what would makes sense for my version of the function 
> either... because RTE_NEXT_ABI is a binary on/off but trying to track a 
> multi-variate quantity of ABI updates.
> 
> For now I guess I have to write it like this inside the RTE_NEXT_ABI:
> 
> rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint32_t *dp, uint32_t ptype)
> 
> This seems unpleasant and kind of painful. What did I miss here?
> 
> Matthew.
> 
> static inline __attribute__((always_inline)) void
> <<< 8e29af8a2843b6342dbc72db43ac82c9d29695bf
> #ifdef RTE_NEXT_ABI
> rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint16_t *dp, uint32_t ptype)
> #else
> rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint16_t *dp, uint32_t flags)
> #endif

The new mbuf provides packet type instead of flags.
So the processing in this function is changed and the variable name is
different to reflect this.

[dpdk-dev] [PATCH v3 1/6] szedata2: add new poll mode driver

[Thomas Monjalon]

2015-11-20 20:25, Matej Vido:
> > As only 64-bit versions of the libraries are provided, I guess we
> > could mention it is currently supported only on x86-64.
> 
> I agree. Should I update the documentation and send a patch?

Yes please.

Another note: I use -rpath= in EXTRA_LDFLAGS to find the dependency of the 
dependency ;)

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B. Melton]

David/Thomas,  in-line  -Roger


On 11/18/15 5:13 PM, Roger B. Melton wrote:
> Hi Thomas, in-line  -Roger
>
> On 11/17/15 10:46 AM, Thomas Monjalon wrote:
>> 2015-11-17 08:56, Roger B. Melton:
>>> Hi David,  in-line -Roger
>>>
>>> On 11/16/15 4:46 AM, David Marchand wrote:
 Hello Roger,

 On Sun, Nov 15, 2015 at 3:45 PM, Roger B. Melton >>> > wrote:

  I like the "-b all" and "-w none" idea, but I think it might be
  complicated to implement it the way we would need it to work.  
 The
  existing -b and -w options  persist for the duration of the
  application, and we would need the "-b all"/"-w none" to persists
  only through rte_eal_init() time.  Otherwise our attempt to to
  attach a device at a later time would be blocked by the option.

 I agree, the black/white lists should only apply to initial scan.
 I forgot about this problem ...
 I had started some cleanup in the pci scan / attach code but this is
 too late for 2.2, I will post this in the next merge window.


  Wouldn't it be simpler to have an option to disable the
  rte_eal_init() time the probe.  Would that address the issue with
  VFIO, prevent automatically attaching to devices while permitting
  on demand attach?


 I suppose we can do this yes (I think Thomas once proposed off-list an
 option like --no-pci-scan).
 Do you think you can send a patch ?
>>> What about --no-pci-init-probe?  I know it's long, but it is more
>>> descriptive of it's purpose to disable only the init time pci probe.
>> Why not a "-b all"?
>> Making it work would also solve the case where you to scan only part of
>> the devices and initialize the blacklisted ones later.
>> .
>>
> Do you envision "-b all" setting a flag that would be used to block 
> rte_eal_init() invocation of rte_eal_pci_probe()?  e.g. If we have a 
> new API, *rte_eal_pci_blacklist_all_get()* that returns a non-zero 
> value if "-b all" was specified, then in rte_eal_init() we would have 
> something like:
>
> ...
> /* Probe & Initialize PCI devices */
> *if (!rte_eal_pci_blacklist_all_get())**  <--- New check*
> if (rte_eal_pci_probe())
> rte_panic("Cannot probe PCI\n");
> ...
>
>
> Or setting a flag that would be checked in rte_eal_probe_one() similar 
> to the existing per device blacklist check?  e.g. Again with a new 
> API, *rte_eal_pci_blacklist_all_get()* that returns a non-zero value 
> if "-b all" was specified, then in rte_eal_pci_probe_one_driver() we 
> would have something like:
>
> /* no initialization when blacklisted, return without error */
> if (*rte_eal_pci_blacklist_all_get() ||  <--- New check*
> (dev->devargs != NULL &&
>  dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)) {
> RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not 
> initializing\n");
> return 1;
> }
>
> The former would work, but I think it would be confusing for "-b all" 
> to only apply to init.
>
> The latter would be consistent with how "-b " works, but in 
> order to initialize devices at a later time, we would need a way to 
> clear the blacklist all state at run time so that 
> *rte_eal_pci_blacklist_all()* would return false. For example, 
> something like *rte_eal_pci_blacklist_all_clear()*.
>
> Or do you have something else in mind entirely?
>
> .
>

How about something like this:

  * Add pci_blacklist_all member to eal internal config.
  * When non-zero, pci_blacklist_all will prevent PCI probing. Result is
that probes will find no match.
  * pci_blacklist_all is temporal
  o Can be set at rte_eal_init() time with "-b all" switch.
  o Can be set or cleared at run time with
rte_eal_pci_blacklist_all_set() or
rte_eal_pci_blacklist_all_clear() respectively.
  o State can be queried with rte_eal_pci_blacklist_all_get()
  * Blacklist and whitelist switches with PCI DBDF are still permitted
as before, but they are used only when the blacklist all override is
cleared.

I've tested the implementation from the diff below in our application 
and it works well.  Note our application falls under linuxapp.  I 
haven't tested bsdapp yet.

If it looks good to you, I will update test application and update if 
necessary with logic to test the capability, and complete testing of the 
bsdapp.

Thanks,  -Roger

On branch DPDK2.2Integration
Changes not staged for commit:
   (use "git add ..." to update what will be committed)
   (use "git checkout -- ..." to discard changes in working directory)

modified:   lib/librte_eal/bsdapp/eal/eal.c
modified:   lib/librte_eal/common/eal_common_options.c
modified:   lib/librte_eal/common/eal_common_pci.c
modified:   lib/librte_eal/common/include/rte_eal.h
modified:   lib/librte_eal/linuxapp/eal/eal.c

no changes added to commit

[dpdk-dev] difficulty w/ RTE_NEXT_ABI

[Matthew Hall]

I was trying to rebase my DPDK onto v2.1.0 and I came across some very 
confusing code in examples/l3fwd/main.c .

So... this code used the RTE_NEXT_ABI macros on a change which does not appear 
to affect the API... on a function that is marked always_inline ???

Maybe I missed something but this seems pointless. An always_inline function 
is going to have to be recompiled in any case.

Now I have no clue what would makes sense for my version of the function 
either... because RTE_NEXT_ABI is a binary on/off but trying to track a 
multi-variate quantity of ABI updates.

For now I guess I have to write it like this inside the RTE_NEXT_ABI:

rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint32_t *dp, uint32_t ptype)

This seems unpleasant and kind of painful. What did I miss here?

Matthew.

static inline __attribute__((always_inline)) void
<<< 8e29af8a2843b6342dbc72db43ac82c9d29695bf
#ifdef RTE_NEXT_ABI
rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint16_t *dp, uint32_t ptype)
#else
rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint16_t *dp, uint32_t flags)
#endif
===
rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint32_t *dp, uint32_t flags)
>>> examples: update examples to use 24 bit extended next hop
{