RE: [PATCH v1] ethdev: add direction info when creating the transfer table

[Ivan Malov <ivan.malov@oktetlabs.ru>]

Hi Rongwei,

In this reply, I do not include the previous mail because the amount
of inline commentary has gone haywire over the past couple of days.
Let's re-iterate.

But before I get to that, I'd like to offer a fresh perspective:

Perhaps, if we all agree that term "vport" means an endpoint which
can stand for any "port" except for physical one, then it should
be possible to use term ANY_VPORTS rather than ANY_GUEST_PORTS.

But that's tricky, of course. I don't have a way with naming,
so more opinions are welcome and very-very desirable here.

So:

1) Do you agree that, in your proposal, the new "wire_orig" / "vf_orig"
    primitives are in fact yet another match criteria?

    ..

    To me, it looks so. If they are match criteria, then they belong
    in match pattern, that is, they should be expressed as new items.

    For "transfer" rules, the *existing* attributes are: "group"
    and "priority". As you may note, these are clearly not match
    criteria. They control the look-up order. So, to this day,
    there're no match criteria in DPDK expressed as attributes.

    If these "wire_orig" / "vf_orig" are going to be introduced
    as attributes, that should be backed with strong motivation.

2) From your viewpoint, why items "ANY_PHYS_PORTS" and "ANY_VPORTS"
    won't do? Or, which problems do you think they may inflict?

    ..

    Previously, you explained why REPRESENTED_PORT would not
    fit your needs. And I understand your point: to async API,
    two pattern templates which both have item REPRESENTED_PORT
    in them cannot be clearly distinguished and are in fact the
    same set of criteria (provided that all other items are also
    the same and have the same masks). Templates are, well,
    templates (or shapes) of the rules to come later and
    do not include exact "spec" for the "ethdev_id".
    Got it.

    But that's not going to be the case with items ANY_PHYS_PORTS and
    ANY_VPORTS, is it? In one async table template, the user submits
    item ANY_PHYS_PORTS (instead of table attribute "wire_orig").
    In another template, the user submits item ANY_VPORTS to
    state that they want to match only traffic transmitted
    software endpoints (DPDK ethdevs, guest VFs, etc.)
    connected to the switch.

    In this example, the PMD will clearly see that the two templates
    differ. So it will be able to allocate separate resources, each
    one "cutting one half of traffic" (as per your concept).

3) In your most recent response, you suggested that one might have
    had the attributes occupied for some other purposes. To me,
    they're not. Neither me nor my closest colleagues have
    any plans on them. When I advocate using item approach
    over the attribute approach, I do this to ensure
    a) clarity of the API contract and b) robustness.

4) Also, in your response, you suggested that I might have
    confused item mask and spec. That is not the case.
    If we agree, that switch domain ID is unneeded in
    the new items, then these items will have no
    fields in them (like item PF had not had any
    before it was deprecated).

    No fields in new items => no field masks.
    So what's the problem then?

5) With regard to our talk about identifying the relationship
    between ethdevs and switch domains, you said that the user
    could know the difference from the very beginning:
    /sysfs/ .... /PF_BDF/sriov_num

    That is true for the user who starts the application, but
    this knowledge is hard to obtain from the application
    perspective = it's hard to automate.

    This is why ethdevs are able to advertise their domain IDs.
    And, as I explained, looking at domain ID to understand
    port relationship is valid, whilst looking at proxy IDs
    to achieve the same goal is not. Proxy port IDs only
    serve the purpose of finding an entry point for
    managing flows. That has slightly different
    meaning, but this subtle difference is important.

6) As for the confusion over the difference between fixing
    bugs and making the code robust by extra checks:

    Yes, I agree that the programmer who writes the
    application must be intelligent enough to use
    flow primitives the proper way. Yes, the user
    who starts the application also should thread
    carefully. But that does not prevent some
    mistakes in other parts of code from
    corrupting various chunks of memory,
    including, for example, flow attrs.

    You say that such mistakes have to be "just fixed"
    as any other bugs. Right. But how much time will
    the programmer spend to identify the bugs?

    If the PMDs do all the checks (as with attributes),
    the hypothetical bug will manifest itself much
    earlier. That will simplify debugging by a lot...

    So, my point is that it's still better to ensure
    that new flow primitives have all necessary
    checks in place. For attributes, it is
    required to add them separately.

    For items, as I explained, it might not be necessary
    in the majority of cases simply because of the
    switch (item->type) { case } structure.

So, these are some of my points to explain why the
attribute approach is untenable. To me, attributes
are something global, which demands checks in all
flow-capable PMDs. Items seem better because they
are don't cares to all PMDs which are unaware of
the async concept. So, even if someone does not
implement the async concept or does not like
the new item names, they can turn a blind
eye to this - with attributes, thay can't.

Thank you.