netdev - Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210316104223.GA16688@netronome.com>
Date:   Tue, 16 Mar 2021 11:42:23 +0100
From:   Simon Horman <simon.horman@...ronome.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com, Xingfeng Hu <xingfeng.hu@...igine.com>,
        Baowen Zheng <baowen.zheng@...igine.com>,
        Louis Peens <louis.peens@...ronome.com>
Subject: Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for
 packet-per-second policing

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.

...