Re: [PATCH net-next] Revert "netdevsim: Add multi-queue support"

On Tue, Aug 3, 2021 at 2:51 PM Jakub Kicinski [off-list ref] wrote:

On Tue, 3 Aug 2021 14:32:19 -0700 Cong Wang wrote:

quoted

On Tue, Aug 3, 2021 at 2:18 PM Jakub Kicinski [off-list ref] wrote:

quoted

On Tue, 3 Aug 2021 10:11:13 -0700 Cong Wang wrote:

quoted

On Tue, Aug 3, 2021 at 5:39 AM Jakub Kicinski [off-list ref] wrote:
Since when netdevsim is *only* for upstream tests?

Since it was created.

Why it was created only for upstream? IOW, what's wrong with
using it only for non-upstream tests?

BTW, we also use dummy device for testing, it is not only for
upstream. It is extremely odd to single netdevsim out. I don't
see any special reason here.

From my own experience companies which are serious about their
engineering have a lot of code dedicated to testing. I don't think
we can deal with all such code upstream.

At the same time I want to incentivize upstreaming all of the tests
which are widely applicable (i.e. not HW-specific).

So, nothing special for netdevsim? This seems applicable to all code,
not just netdevsim code.

Last but not least test harnesses are really weird from functional, code
lifetime and refactoring perspective. netdevsim is not expected to keep
uAPI as long as in-tree tests do no break/are updated as well.

Sure. Our test is not any special, sch_mq is in upstream, only sch_mq
tests are not yet. Peilin will send out sch_mq tests very soon.

quoted

quoted

quoted

Even if so, where is this documented? And why not just point it
out when reviewing it instead of silently waiting for weeks?

I was AFK for the last two weeks.

How about documenting it in netdev-FAQ (or literally any doc)?
This would save everyone's time.

Fair, I'll send a patch.

Great! Really appreciate it.

quoted

quoted

quoted

It is clearly not dead. We internally used it for testing sch_mq,
this is clearly stated in the git log.

Please contribute those tests upstream or keep any test harness
they require where such test are, out of tree.

Peilin will add tc-testing for sch_mq which requires this netdevsim
feature.

quoted

quoted

How did you draw such a conclusion without talking to authors?

There is no upstream test using this code, and I did CC you, didn't I?

There are downstream tests, which are mentioned in changelog.

I am pretty sure upstream tests only cover part of the whole networking
code, if you really want to apply the rule, a lot of code are already dead.
Once again, I don't see any reason why you only treat netdevsim differently.
;)

I hope the first part of this response scheds some light.

quoted

quoted

quoted

But this does remind me of using netdevsim for tc-testing.

Please bring the code back as part of the series adding upstream tests.

Please remove all those not covered by upstream tests just to be fair??

I'd love to remove all test harnesses upstream which are not used by
upstream tests, sure :)

Many net/*/ code can be gone. Maybe start with net/netrom/? ;)

Thanks.