Hi Ido, On Tue, Mar 16, 2021 at 10:35:35AM +0200, Ido Schimmel wrote: > Sorry for the delay. Was AFK yesterday
No problem at all. ... > > > > As follow-ups we plan to provide: > > > > * Corresponding updates to iproute2 > > > > * Corresponding self tests (which depend on the iproute2 changes) > > > > > > I was about to ask :) > > > > > > FYI, there is this selftest: > > > tools/testing/selftests/net/forwarding/tc_police.sh > > > > > > Which can be extended to also test packet rate policing > > > > Thanks Ido, > > > > The approach we have taken is to add tests to > > tools/testing/selftests/tc-testing/tc-tests/actions/police.json > > > > Do you think adding a test to tc_police.sh is also worthwhile? Or should be > > done instead of updating police.json? > > IIUC, police.json only performs configuration tests. tc_police.sh on the > other hand, configures a topology, injects traffic and validates that > the bandwidth after the police action is according to user > configuration. You can test the software data path by using veth pairs > or the hardware data path by using physical ports looped to each other. > > So I think that extending both tests is worthwhile. Thanks, we'll see about making it so. > > Lastly, my assumption is that the tests should be posted once iproute2 > > changes they depend on have been accepted. Is this correct in your opinion? > > Personally, I prefer selftests to be posted together with the > implementation, regardless if they depend on new iproute2 functionality. > In the unlikely case that the kernel patches were accepted, but changes > were requested for the command line interface, you can always patch the > selftests later. > > Jakub recently added this section: > https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#how-do-i-post-corresponding-changes-to-user-space-components > > He writes "User space code exercising kernel features should be posted > alongside kernel patches." > > And you can see that in the example the last patch is a selftest: > > ``` > [PATCH net-next 0/3] net: some feature cover letter > └─ [PATCH net-next 1/3] net: some feature prep > └─ [PATCH net-next 2/3] net: some feature do it > └─ [PATCH net-next 3/3] selftest: net: some feature > > [PATCH iproute2-next] ip: add support for some feature > ``` Thanks, we'll try to follow this in our next feature submission. ...
