| Message ID | cover.1610978306.git.petrm@nvidia.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | nexthop: More fine-grained policies for netlink message validation | expand |
On 1/18/21 7:05 AM, Petr Machata wrote: > From: Petr Machata <petrm@nvidia.org> > > There is currently one policy that covers all attributes for next hop > object management. Actual validation is then done in code, which makes it > unobvious which attributes are acceptable when, and indeed that everything > is rejected as necessary. > > In this series, split rtm_nh_policy to several policies that cover various > aspects of the next hop object configuration, and instead of open-coding > the validation, defer to nlmsg_parse(). This should make extending the next > hop code simpler as well, which will be relevant in near future for > resilient hashing implementation. > > This was tested by running tools/testing/selftests/net/fib_nexthops.sh. > Additionally iproute2 was tweaked to issue "nexthop list id" as an > RTM_GETNEXTHOP dump request, instead of a straight get to test that > unexpected attributes are indeed rejected. > > In patch #1, convert attribute validation in nh_valid_get_del_req(). > > In patch #2, convert nh_valid_dump_req(). > > In patch #3, rtm_nh_policy is cleaned up and renamed to rtm_nh_policy_new, > because after the above two patches, that is the only context that it is > used in. > > Petr Machata (3): > nexthop: Use a dedicated policy for nh_valid_get_del_req() > nexthop: Use a dedicated policy for nh_valid_dump_req() > nexthop: Specialize rtm_nh_policy > > net/ipv4/nexthop.c | 85 +++++++++++++++++----------------------------- > 1 file changed, 32 insertions(+), 53 deletions(-) > good cleanup. thanks for doing this. Did you run fib_nexthops.sh selftests on the change? Seems right, but always good to run that script which has functional tests about valid attribute combinations.
On Mon, Jan 18, 2021 at 10:43:22AM -0700, David Ahern wrote: > On 1/18/21 7:05 AM, Petr Machata wrote: > > From: Petr Machata <petrm@nvidia.org> > > > > There is currently one policy that covers all attributes for next hop > > object management. Actual validation is then done in code, which makes it > > unobvious which attributes are acceptable when, and indeed that everything > > is rejected as necessary. > > > > In this series, split rtm_nh_policy to several policies that cover various > > aspects of the next hop object configuration, and instead of open-coding > > the validation, defer to nlmsg_parse(). This should make extending the next > > hop code simpler as well, which will be relevant in near future for > > resilient hashing implementation. > > > > This was tested by running tools/testing/selftests/net/fib_nexthops.sh. > > Additionally iproute2 was tweaked to issue "nexthop list id" as an > > RTM_GETNEXTHOP dump request, instead of a straight get to test that > > unexpected attributes are indeed rejected. > > > > In patch #1, convert attribute validation in nh_valid_get_del_req(). > > > > In patch #2, convert nh_valid_dump_req(). > > > > In patch #3, rtm_nh_policy is cleaned up and renamed to rtm_nh_policy_new, > > because after the above two patches, that is the only context that it is > > used in. > > > > Petr Machata (3): > > nexthop: Use a dedicated policy for nh_valid_get_del_req() > > nexthop: Use a dedicated policy for nh_valid_dump_req() > > nexthop: Specialize rtm_nh_policy > > > > net/ipv4/nexthop.c | 85 +++++++++++++++++----------------------------- > > 1 file changed, 32 insertions(+), 53 deletions(-) > > > > good cleanup. thanks for doing this. Did you run fib_nexthops.sh > selftests on the change? Seems right, but always good to run that script > which has functional tests about valid attribute combinations. "This was tested by running tools/testing/selftests/net/fib_nexthops.sh" :)
From: Petr Machata <petrm@nvidia.org> There is currently one policy that covers all attributes for next hop object management. Actual validation is then done in code, which makes it unobvious which attributes are acceptable when, and indeed that everything is rejected as necessary. In this series, split rtm_nh_policy to several policies that cover various aspects of the next hop object configuration, and instead of open-coding the validation, defer to nlmsg_parse(). This should make extending the next hop code simpler as well, which will be relevant in near future for resilient hashing implementation. This was tested by running tools/testing/selftests/net/fib_nexthops.sh. Additionally iproute2 was tweaked to issue "nexthop list id" as an RTM_GETNEXTHOP dump request, instead of a straight get to test that unexpected attributes are indeed rejected. In patch #1, convert attribute validation in nh_valid_get_del_req(). In patch #2, convert nh_valid_dump_req(). In patch #3, rtm_nh_policy is cleaned up and renamed to rtm_nh_policy_new, because after the above two patches, that is the only context that it is used in. Petr Machata (3): nexthop: Use a dedicated policy for nh_valid_get_del_req() nexthop: Use a dedicated policy for nh_valid_dump_req() nexthop: Specialize rtm_nh_policy net/ipv4/nexthop.c | 85 +++++++++++++++++----------------------------- 1 file changed, 32 insertions(+), 53 deletions(-)