On Monday 09 August 2021 12:25:46 Jakub Kicinski wrote:
> On Sat, 7 Aug 2021 18:37:49 +0200 Pali Rohár wrote:
> > Currently there are two ways how to create a new ppp interface. Old method
> > via ioctl(PPPIOCNEWUNIT) and new method via rtnl RTM_NEWLINK/NLM_F_CREATE
> > which was introduced in v4.7 by commit 96d934c70db6 ("ppp: add rtnetlink
> > device creation support").
> >
> > ...
>
> Your 2 previous patches were fixes and went into net, this patch seems
> to be on top of them but is a feature, so should go to net-next.
Yes
> But it doesn't apply to net-next given net was not merged into net-next.
> Please rebase on top of net-next or (preferably) wait until next week
> so that the trees can get merged and then you can repost without causing
> any conflicts.
Better to wait. I would like hear some comments / review on this patch
if this is the correct approach as it adds a new API/ABI for userspace.
> > static const struct nla_policy ppp_nl_policy[IFLA_PPP_MAX + 1] = {
> > [IFLA_PPP_DEV_FD] = { .type = NLA_S32 },
> > + [IFLA_PPP_UNIT_ID] = { .type = NLA_S32 },
> > };
>
> set .strict_start_type, please so new attrs get validated better
>
> > static int ppp_nl_validate(struct nlattr *tb[], struct nlattr *data[],
> > @@ -1274,6 +1277,15 @@ static int ppp_nl_validate(struct nlattr *tb[], struct nlattr *data[],
> >
> > if (!data[IFLA_PPP_DEV_FD])
> > return -EINVAL;
> > +
> > + /* Check for IFLA_PPP_UNIT_ID before IFLA_PPP_DEV_FD to allow userspace
> > + * detect if kernel supports IFLA_PPP_UNIT_ID or not by specifying
> > + * negative IFLA_PPP_DEV_FD. Previous kernel versions ignored
> > + * IFLA_PPP_UNIT_ID attribute.
> > + */
> > + if (data[IFLA_PPP_UNIT_ID] && nla_get_s32(data[IFLA_PPP_UNIT_ID]) < -1)
> > + return -EINVAL;
>
> please use NLA_POLICY_MIN() instead, no need to open-code
>
> > if (nla_get_s32(data[IFLA_PPP_DEV_FD]) < 0)
> > return -EBADF;
I will look at both issues... and I would like to know what is preferred
way to introduce new attributes in a way that userspace can detect if
kernel supports them or not.
