* [PATCH bpf-next 0/2] netkit: two minor cleanups
@ 2023-10-26 9:41 Nikolay Aleksandrov
2023-10-26 9:41 ` [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization Nikolay Aleksandrov
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2023-10-26 9:41 UTC (permalink / raw)
To: bpf
Cc: jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel, Nikolay Aleksandrov
Hi,
This set does two minor cleanups mentioned by Jiri. The first patch
removes explicit NULLing of primary/peer pointers and relies on the
implicit mem zeroing done at net device alloc. The second patch switches
netkit's mode and primary/peer policy netlink attributes to use
NLA_POLICY_VALIDATE_FN() type and sets the custom validate function to
return better user errors. This way netlink's policy is used to validate
the attributes and simplifies the code a bit. No functional changes are
intended.
Thanks,
Nik
Nikolay Aleksandrov (2):
netkit: remove explicit active/peer ptr initialization
netkit: use netlink policy for mode and policy attributes validation
drivers/net/netkit.c | 70 ++++++++++++++------------------------------
1 file changed, 22 insertions(+), 48 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization
2023-10-26 9:41 [PATCH bpf-next 0/2] netkit: two minor cleanups Nikolay Aleksandrov
@ 2023-10-26 9:41 ` Nikolay Aleksandrov
2023-10-26 12:12 ` Daniel Borkmann
2023-10-26 13:21 ` Jiri Pirko
2023-10-26 9:41 ` [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation Nikolay Aleksandrov
2023-10-26 14:10 ` [PATCH bpf-next 0/2] netkit: two minor cleanups patchwork-bot+netdevbpf
2 siblings, 2 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2023-10-26 9:41 UTC (permalink / raw)
To: bpf
Cc: jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel, Nikolay Aleksandrov
Remove the explicit NULLing of active/peer pointers and rely on the
implicit one done at net device allocation.
Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
drivers/net/netkit.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
index 7e484f9fd3ae..5a0f86f38f09 100644
--- a/drivers/net/netkit.c
+++ b/drivers/net/netkit.c
@@ -371,8 +371,6 @@ static int netkit_new_link(struct net *src_net, struct net_device *dev,
nk->policy = default_peer;
nk->mode = mode;
bpf_mprog_bundle_init(&nk->bundle);
- RCU_INIT_POINTER(nk->active, NULL);
- RCU_INIT_POINTER(nk->peer, NULL);
err = register_netdevice(peer);
put_net(net);
@@ -398,8 +396,6 @@ static int netkit_new_link(struct net *src_net, struct net_device *dev,
nk->policy = default_prim;
nk->mode = mode;
bpf_mprog_bundle_init(&nk->bundle);
- RCU_INIT_POINTER(nk->active, NULL);
- RCU_INIT_POINTER(nk->peer, NULL);
err = register_netdevice(dev);
if (err < 0)
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation
2023-10-26 9:41 [PATCH bpf-next 0/2] netkit: two minor cleanups Nikolay Aleksandrov
2023-10-26 9:41 ` [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization Nikolay Aleksandrov
@ 2023-10-26 9:41 ` Nikolay Aleksandrov
2023-10-26 12:13 ` Daniel Borkmann
` (2 more replies)
2023-10-26 14:10 ` [PATCH bpf-next 0/2] netkit: two minor cleanups patchwork-bot+netdevbpf
2 siblings, 3 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2023-10-26 9:41 UTC (permalink / raw)
To: bpf
Cc: jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel, Nikolay Aleksandrov
Use netlink's NLA_POLICY_VALIDATE_FN() type for mode and primary/peer
policy with custom validation functions to return better errors. This
simplifies the logic a bit and relies on netlink's policy validation.
We don't have to specify len because the type is NLA_U32 and attribute
length is enforced by netlink.
Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
drivers/net/netkit.c | 66 +++++++++++++++-----------------------------
1 file changed, 22 insertions(+), 44 deletions(-)
diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
index 5a0f86f38f09..1ce116e68f95 100644
--- a/drivers/net/netkit.c
+++ b/drivers/net/netkit.c
@@ -247,29 +247,29 @@ static struct net *netkit_get_link_net(const struct net_device *dev)
return peer ? dev_net(peer) : dev_net(dev);
}
-static int netkit_check_policy(int policy, struct nlattr *tb,
+static int netkit_check_policy(const struct nlattr *attr,
struct netlink_ext_ack *extack)
{
- switch (policy) {
+ switch (nla_get_u32(attr)) {
case NETKIT_PASS:
case NETKIT_DROP:
return 0;
default:
- NL_SET_ERR_MSG_ATTR(extack, tb,
+ NL_SET_ERR_MSG_ATTR(extack, attr,
"Provided default xmit policy not supported");
return -EINVAL;
}
}
-static int netkit_check_mode(int mode, struct nlattr *tb,
+static int netkit_check_mode(const struct nlattr *attr,
struct netlink_ext_ack *extack)
{
- switch (mode) {
+ switch (nla_get_u32(attr)) {
case NETKIT_L2:
case NETKIT_L3:
return 0;
default:
- NL_SET_ERR_MSG_ATTR(extack, tb,
+ NL_SET_ERR_MSG_ATTR(extack, attr,
"Provided device mode can only be L2 or L3");
return -EINVAL;
}
@@ -306,13 +306,8 @@ static int netkit_new_link(struct net *src_net, struct net_device *dev,
int err;
if (data) {
- if (data[IFLA_NETKIT_MODE]) {
- attr = data[IFLA_NETKIT_MODE];
- mode = nla_get_u32(attr);
- err = netkit_check_mode(mode, attr, extack);
- if (err < 0)
- return err;
- }
+ if (data[IFLA_NETKIT_MODE])
+ mode = nla_get_u32(data[IFLA_NETKIT_MODE]);
if (data[IFLA_NETKIT_PEER_INFO]) {
attr = data[IFLA_NETKIT_PEER_INFO];
ifmp = nla_data(attr);
@@ -324,20 +319,10 @@ static int netkit_new_link(struct net *src_net, struct net_device *dev,
return err;
tbp = peer_tb;
}
- if (data[IFLA_NETKIT_POLICY]) {
- attr = data[IFLA_NETKIT_POLICY];
- default_prim = nla_get_u32(attr);
- err = netkit_check_policy(default_prim, attr, extack);
- if (err < 0)
- return err;
- }
- if (data[IFLA_NETKIT_PEER_POLICY]) {
- attr = data[IFLA_NETKIT_PEER_POLICY];
- default_peer = nla_get_u32(attr);
- err = netkit_check_policy(default_peer, attr, extack);
- if (err < 0)
- return err;
- }
+ if (data[IFLA_NETKIT_POLICY])
+ default_prim = nla_get_u32(data[IFLA_NETKIT_POLICY]);
+ if (data[IFLA_NETKIT_PEER_POLICY])
+ default_peer = nla_get_u32(data[IFLA_NETKIT_PEER_POLICY]);
}
if (ifmp && tbp[IFLA_IFNAME]) {
@@ -818,8 +803,6 @@ static int netkit_change_link(struct net_device *dev, struct nlattr *tb[],
struct netkit *nk = netkit_priv(dev);
struct net_device *peer = rtnl_dereference(nk->peer);
enum netkit_action policy;
- struct nlattr *attr;
- int err;
if (!nk->primary) {
NL_SET_ERR_MSG(extack,
@@ -834,22 +817,14 @@ static int netkit_change_link(struct net_device *dev, struct nlattr *tb[],
}
if (data[IFLA_NETKIT_POLICY]) {
- attr = data[IFLA_NETKIT_POLICY];
- policy = nla_get_u32(attr);
- err = netkit_check_policy(policy, attr, extack);
- if (err)
- return err;
+ policy = nla_get_u32(data[IFLA_NETKIT_POLICY]);
WRITE_ONCE(nk->policy, policy);
}
if (data[IFLA_NETKIT_PEER_POLICY]) {
- err = -EOPNOTSUPP;
- attr = data[IFLA_NETKIT_PEER_POLICY];
- policy = nla_get_u32(attr);
- if (peer)
- err = netkit_check_policy(policy, attr, extack);
- if (err)
- return err;
+ if (!peer)
+ return -EOPNOTSUPP;
+ policy = nla_get_u32(data[IFLA_NETKIT_PEER_POLICY]);
nk = netkit_priv(peer);
WRITE_ONCE(nk->policy, policy);
}
@@ -889,9 +864,12 @@ static int netkit_fill_info(struct sk_buff *skb, const struct net_device *dev)
static const struct nla_policy netkit_policy[IFLA_NETKIT_MAX + 1] = {
[IFLA_NETKIT_PEER_INFO] = { .len = sizeof(struct ifinfomsg) },
- [IFLA_NETKIT_POLICY] = { .type = NLA_U32 },
- [IFLA_NETKIT_MODE] = { .type = NLA_U32 },
- [IFLA_NETKIT_PEER_POLICY] = { .type = NLA_U32 },
+ [IFLA_NETKIT_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
+ netkit_check_policy),
+ [IFLA_NETKIT_MODE] = NLA_POLICY_VALIDATE_FN(NLA_U32,
+ netkit_check_mode),
+ [IFLA_NETKIT_PEER_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
+ netkit_check_policy),
[IFLA_NETKIT_PRIMARY] = { .type = NLA_REJECT,
.reject_message = "Primary attribute is read-only" },
};
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization
2023-10-26 9:41 ` [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization Nikolay Aleksandrov
@ 2023-10-26 12:12 ` Daniel Borkmann
2023-10-26 13:21 ` Jiri Pirko
1 sibling, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2023-10-26 12:12 UTC (permalink / raw)
To: Nikolay Aleksandrov, bpf
Cc: jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf
On 10/26/23 11:41 AM, Nikolay Aleksandrov wrote:
> Remove the explicit NULLing of active/peer pointers and rely on the
> implicit one done at net device allocation.
>
> Suggested-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation
2023-10-26 9:41 ` [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation Nikolay Aleksandrov
@ 2023-10-26 12:13 ` Daniel Borkmann
2023-10-26 13:23 ` Jiri Pirko
2023-10-26 14:11 ` Ido Schimmel
2 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2023-10-26 12:13 UTC (permalink / raw)
To: Nikolay Aleksandrov, bpf
Cc: jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf
On 10/26/23 11:41 AM, Nikolay Aleksandrov wrote:
> Use netlink's NLA_POLICY_VALIDATE_FN() type for mode and primary/peer
> policy with custom validation functions to return better errors. This
> simplifies the logic a bit and relies on netlink's policy validation.
> We don't have to specify len because the type is NLA_U32 and attribute
> length is enforced by netlink.
>
> Suggested-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> drivers/net/netkit.c | 66 +++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 44 deletions(-)
Looks better indeed, shrinks code a bit, and passes the selftests, thanks
for the suggestion!
Thanks,
Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization
2023-10-26 9:41 ` [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization Nikolay Aleksandrov
2023-10-26 12:12 ` Daniel Borkmann
@ 2023-10-26 13:21 ` Jiri Pirko
1 sibling, 0 replies; 12+ messages in thread
From: Jiri Pirko @ 2023-10-26 13:21 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: bpf, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel
Thu, Oct 26, 2023 at 11:41:05AM CEST, razor@blackwall.org wrote:
>Remove the explicit NULLing of active/peer pointers and rely on the
>implicit one done at net device allocation.
>
>Suggested-by: Jiri Pirko <jiri@resnulli.us>
>Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation
2023-10-26 9:41 ` [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation Nikolay Aleksandrov
2023-10-26 12:13 ` Daniel Borkmann
@ 2023-10-26 13:23 ` Jiri Pirko
2023-10-26 14:11 ` Ido Schimmel
2 siblings, 0 replies; 12+ messages in thread
From: Jiri Pirko @ 2023-10-26 13:23 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: bpf, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel
Thu, Oct 26, 2023 at 11:41:06AM CEST, razor@blackwall.org wrote:
>Use netlink's NLA_POLICY_VALIDATE_FN() type for mode and primary/peer
>policy with custom validation functions to return better errors. This
>simplifies the logic a bit and relies on netlink's policy validation.
>We don't have to specify len because the type is NLA_U32 and attribute
>length is enforced by netlink.
>
>Suggested-by: Jiri Pirko <jiri@resnulli.us>
>Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/2] netkit: two minor cleanups
2023-10-26 9:41 [PATCH bpf-next 0/2] netkit: two minor cleanups Nikolay Aleksandrov
2023-10-26 9:41 ` [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization Nikolay Aleksandrov
2023-10-26 9:41 ` [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation Nikolay Aleksandrov
@ 2023-10-26 14:10 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-10-26 14:10 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: bpf, jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel
Hello:
This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Thu, 26 Oct 2023 12:41:04 +0300 you wrote:
> Hi,
> This set does two minor cleanups mentioned by Jiri. The first patch
> removes explicit NULLing of primary/peer pointers and relies on the
> implicit mem zeroing done at net device alloc. The second patch switches
> netkit's mode and primary/peer policy netlink attributes to use
> NLA_POLICY_VALIDATE_FN() type and sets the custom validate function to
> return better user errors. This way netlink's policy is used to validate
> the attributes and simplifies the code a bit. No functional changes are
> intended.
>
> [...]
Here is the summary with links:
- [bpf-next,1/2] netkit: remove explicit active/peer ptr initialization
https://git.kernel.org/bpf/bpf-next/c/ea41b880cc85
- [bpf-next,2/2] netkit: use netlink policy for mode and policy attributes validation
https://git.kernel.org/bpf/bpf-next/c/3de07b963ab8
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation
2023-10-26 9:41 ` [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation Nikolay Aleksandrov
2023-10-26 12:13 ` Daniel Borkmann
2023-10-26 13:23 ` Jiri Pirko
@ 2023-10-26 14:11 ` Ido Schimmel
2023-10-26 14:23 ` Nikolay Aleksandrov
2 siblings, 1 reply; 12+ messages in thread
From: Ido Schimmel @ 2023-10-26 14:11 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: bpf, jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel
On Thu, Oct 26, 2023 at 12:41:06PM +0300, Nikolay Aleksandrov wrote:
> static const struct nla_policy netkit_policy[IFLA_NETKIT_MAX + 1] = {
> [IFLA_NETKIT_PEER_INFO] = { .len = sizeof(struct ifinfomsg) },
> - [IFLA_NETKIT_POLICY] = { .type = NLA_U32 },
> - [IFLA_NETKIT_MODE] = { .type = NLA_U32 },
> - [IFLA_NETKIT_PEER_POLICY] = { .type = NLA_U32 },
> + [IFLA_NETKIT_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
> + netkit_check_policy),
Nik, it's problematic to use NLA_POLICY_VALIDATE_FN() with anything
other than NLA_BINARY. See commit 9e17f99220d1 ("net/sched: act_mpls:
Fix warning during failed attribute validation").
> + [IFLA_NETKIT_MODE] = NLA_POLICY_VALIDATE_FN(NLA_U32,
> + netkit_check_mode),
> + [IFLA_NETKIT_PEER_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
> + netkit_check_policy),
> [IFLA_NETKIT_PRIMARY] = { .type = NLA_REJECT,
> .reject_message = "Primary attribute is read-only" },
> };
> --
> 2.38.1
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation
2023-10-26 14:11 ` Ido Schimmel
@ 2023-10-26 14:23 ` Nikolay Aleksandrov
2023-10-26 14:25 ` Daniel Borkmann
2023-10-26 14:34 ` Nikolay Aleksandrov
0 siblings, 2 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2023-10-26 14:23 UTC (permalink / raw)
To: Ido Schimmel
Cc: bpf, jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel
On 10/26/23 17:11, Ido Schimmel wrote:
> On Thu, Oct 26, 2023 at 12:41:06PM +0300, Nikolay Aleksandrov wrote:
>> static const struct nla_policy netkit_policy[IFLA_NETKIT_MAX + 1] = {
>> [IFLA_NETKIT_PEER_INFO] = { .len = sizeof(struct ifinfomsg) },
>> - [IFLA_NETKIT_POLICY] = { .type = NLA_U32 },
>> - [IFLA_NETKIT_MODE] = { .type = NLA_U32 },
>> - [IFLA_NETKIT_PEER_POLICY] = { .type = NLA_U32 },
>> + [IFLA_NETKIT_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
>> + netkit_check_policy),
>
> Nik, it's problematic to use NLA_POLICY_VALIDATE_FN() with anything
> other than NLA_BINARY. See commit 9e17f99220d1 ("net/sched: act_mpls:
> Fix warning during failed attribute validation").
>
But how is that code called at all? The validation type is
NLA_VALIDATE_FUNCTION(), not NLA_VALIDATE_MIN/MAX/RANGE/RANGE_WARN...
nla_validate_int_range() is called only on:
case NLA_VALIDATE_RANGE_PTR:
case NLA_VALIDATE_RANGE:
case NLA_VALIDATE_RANGE_WARN_TOO_LONG:
case NLA_VALIDATE_MIN:
case NLA_VALIDATE_MAX:
Anyway, I'll switch to NLA_BINARY in a bit to make sure it's ok. Thanks
for the pointer.
>> + [IFLA_NETKIT_MODE] = NLA_POLICY_VALIDATE_FN(NLA_U32,
>> + netkit_check_mode),
>> + [IFLA_NETKIT_PEER_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
>> + netkit_check_policy),
>> [IFLA_NETKIT_PRIMARY] = { .type = NLA_REJECT,
>> .reject_message = "Primary attribute is read-only" },
>> };
>> --
>> 2.38.1
>>
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation
2023-10-26 14:23 ` Nikolay Aleksandrov
@ 2023-10-26 14:25 ` Daniel Borkmann
2023-10-26 14:34 ` Nikolay Aleksandrov
1 sibling, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2023-10-26 14:25 UTC (permalink / raw)
To: Nikolay Aleksandrov, Ido Schimmel
Cc: bpf, jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf
On 10/26/23 4:23 PM, Nikolay Aleksandrov wrote:
> On 10/26/23 17:11, Ido Schimmel wrote:
>> On Thu, Oct 26, 2023 at 12:41:06PM +0300, Nikolay Aleksandrov wrote:
>>> static const struct nla_policy netkit_policy[IFLA_NETKIT_MAX + 1] = {
>>> [IFLA_NETKIT_PEER_INFO] = { .len = sizeof(struct ifinfomsg) },
>>> - [IFLA_NETKIT_POLICY] = { .type = NLA_U32 },
>>> - [IFLA_NETKIT_MODE] = { .type = NLA_U32 },
>>> - [IFLA_NETKIT_PEER_POLICY] = { .type = NLA_U32 },
>>> + [IFLA_NETKIT_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
>>> + netkit_check_policy),
>>
>> Nik, it's problematic to use NLA_POLICY_VALIDATE_FN() with anything
>> other than NLA_BINARY. See commit 9e17f99220d1 ("net/sched: act_mpls:
>> Fix warning during failed attribute validation").
>
> But how is that code called at all? The validation type is NLA_VALIDATE_FUNCTION(), not NLA_VALIDATE_MIN/MAX/RANGE/RANGE_WARN...
> nla_validate_int_range() is called only on:
> case NLA_VALIDATE_RANGE_PTR:
> case NLA_VALIDATE_RANGE:
> case NLA_VALIDATE_RANGE_WARN_TOO_LONG:
> case NLA_VALIDATE_MIN:
> case NLA_VALIDATE_MAX:
>
> Anyway, I'll switch to NLA_BINARY in a bit to make sure it's ok. Thanks for the pointer.
Sg, I took in the NULL removal one for now.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation
2023-10-26 14:23 ` Nikolay Aleksandrov
2023-10-26 14:25 ` Daniel Borkmann
@ 2023-10-26 14:34 ` Nikolay Aleksandrov
1 sibling, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2023-10-26 14:34 UTC (permalink / raw)
To: Ido Schimmel
Cc: bpf, jiri, netdev, martin.lau, ast, andrii, john.fastabend, kuba,
andrew, toke, toke, sdf, daniel
On 10/26/23 17:23, Nikolay Aleksandrov wrote:
> On 10/26/23 17:11, Ido Schimmel wrote:
>> On Thu, Oct 26, 2023 at 12:41:06PM +0300, Nikolay Aleksandrov wrote:
>>> static const struct nla_policy netkit_policy[IFLA_NETKIT_MAX + 1] = {
>>> [IFLA_NETKIT_PEER_INFO] = { .len = sizeof(struct
>>> ifinfomsg) },
>>> - [IFLA_NETKIT_POLICY] = { .type = NLA_U32 },
>>> - [IFLA_NETKIT_MODE] = { .type = NLA_U32 },
>>> - [IFLA_NETKIT_PEER_POLICY] = { .type = NLA_U32 },
>>> + [IFLA_NETKIT_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
>>> + netkit_check_policy),
>>
>> Nik, it's problematic to use NLA_POLICY_VALIDATE_FN() with anything
>> other than NLA_BINARY. See commit 9e17f99220d1 ("net/sched: act_mpls:
>> Fix warning during failed attribute validation").
>>
>
> But how is that code called at all? The validation type is
> NLA_VALIDATE_FUNCTION(), not NLA_VALIDATE_MIN/MAX/RANGE/RANGE_WARN...
> nla_validate_int_range() is called only on:
> case NLA_VALIDATE_RANGE_PTR:
> case NLA_VALIDATE_RANGE:
> case NLA_VALIDATE_RANGE_WARN_TOO_LONG:
> case NLA_VALIDATE_MIN:
> case NLA_VALIDATE_MAX:
>
Ah, I'm looking at the wrong thing.. I saw the problem. :)
> Anyway, I'll switch to NLA_BINARY in a bit to make sure it's ok. Thanks
> for the pointer.
>
>>> + [IFLA_NETKIT_MODE] = NLA_POLICY_VALIDATE_FN(NLA_U32,
>>> + netkit_check_mode),
>>> + [IFLA_NETKIT_PEER_POLICY] = NLA_POLICY_VALIDATE_FN(NLA_U32,
>>> + netkit_check_policy),
>>> [IFLA_NETKIT_PRIMARY] = { .type = NLA_REJECT,
>>> .reject_message = "Primary attribute is
>>> read-only" },
>>> };
>>> --
>>> 2.38.1
>>>
>>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-10-26 14:34 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-26 9:41 [PATCH bpf-next 0/2] netkit: two minor cleanups Nikolay Aleksandrov
2023-10-26 9:41 ` [PATCH bpf-next 1/2] netkit: remove explicit active/peer ptr initialization Nikolay Aleksandrov
2023-10-26 12:12 ` Daniel Borkmann
2023-10-26 13:21 ` Jiri Pirko
2023-10-26 9:41 ` [PATCH bpf-next 2/2] netkit: use netlink policy for mode and policy attributes validation Nikolay Aleksandrov
2023-10-26 12:13 ` Daniel Borkmann
2023-10-26 13:23 ` Jiri Pirko
2023-10-26 14:11 ` Ido Schimmel
2023-10-26 14:23 ` Nikolay Aleksandrov
2023-10-26 14:25 ` Daniel Borkmann
2023-10-26 14:34 ` Nikolay Aleksandrov
2023-10-26 14:10 ` [PATCH bpf-next 0/2] netkit: two minor cleanups patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).