On Mon, May 25, 2020 at 2:59 PM Bruce Richardson
[off-list ref] wrote:

On Sun, May 24, 2020 at 06:13:22PM +0200, Thomas Monjalon wrote:

quoted

Bruce, as maintainer of l2fwd example, any opinion about this change?

Assuming all previous discussion on it is resolved, I'm fine with this
patch, though I suspect it will only make 20.08 now.

Acked-by: Bruce Richardson <redacted>

Ping for merge.

quoted

11/05/2020 02:23, Pavan Nikhilesh Bhagavatula:

quoted

Hi Vipin,

quoted

Hi Pavan,

snipped

quoted

quoted

Should we check & warn the user if
1. port speed mismatch
2. on different NUMA
3. port pairs are physical and vdev like tap, and KNI (performance).

Sure, it can be a separate patch as it will be applicable for multiple

examples.
I believe this patch is for example `l2fwd`. But you would like to have to
updated for all `example`. I am ok for this.

snipped

quoted

quoted

Should not the check_port_pair be after this? If the port is not
enabled in port_mask will you skip that pair? or skip RX-TX from that

port?

quoted

We check every port pair against l2fwd_enabled_port_mask in
check_port_pair_config()

quoted

snipped

quoted

quoted

As mentioned above there can ports in mask which might be

disabled for

quoted

quoted

port pair. Should not that be skipped rather than setting last port rx-
tx loopback?

There could be scenarios where user might want to test 2x10G and

1x40G Why

quoted

force the user to explicitly mention 1x40G as port pair of itself in the

portpair

quoted

config?

I am not sure if I follow your thought, as your current port map only
allows `1:1` mapping by `struct port_pair_params`. This can be to self
like `(port0:port0),(port1:port1)` or `(port-0:port-1)`.

1. But current `l2fwd_parse_port_pair_config` does not consider the
same port mapping as we have hard check for `if (nb_port_pair_params

quoted

= RTE_MAX_ETHPORTS/2)`.

2. `l2fwd_enabled_port_mask` is global variable of user port mask. This
can contain both valid and invalid mask. Hence we check
`l2fwd_enabled_port_mask & ~((1 << nb_ports) - 1)`.

3. can these scenarios are true if we invoke `check_port_pair_config`
before actual port_mask check.
a. there are only 4 ports, hence possible mask is `0xf`.
b. user passes port argument as `0xe`
c. `check_port_pair_config` gets masks for `(1,3)` as input and
populates `port_pair_config_mask`.
d.  As per the code, port 2 which is valid port and part of user port mask
will have lastport (which is port 3)? May be I did understand the logic
correct. Can you help me?

Here user needs to explicitly mention (2,2) for port 2 to be setup else it
will be skipped.
If you see `check_port_pair_config` below we disable the ports that are not
Mentioned in portmap.

"
check_port_pair_config(void)
{

<snip>
            port_pair_config_mask |= port_pair_mask;
    }

    l2fwd_enabled_port_mask &= port_pair_config_mask;

    return 0;
}
"

quoted

So my concerns are 1) there is no same port mapping, 2) my
understanding on lastport logic is not clear and 3) as per the code there
is 1:N but 1:1.

Hence there should be sufficient warning to user if port are of wrong
speed and NUMA.

Unless the user disables stats using -T 0 option all the prints will be skipped.

quoted

Note: current speed can be fetched only if the port are started too (in
Fortville).

snipped