All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Pali Rohár" <pali@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>,
	"David S. Miller" <davem@davemloft.net>,
	Guillaume Nault <gnault@redhat.com>,
	linux-ppp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ppp: Add rtnl attribute IFLA_PPP_UNIT_ID for specifying ppp unit id
Date: Mon, 9 Aug 2021 12:25:46 -0700	[thread overview]
Message-ID: <20210809122546.758e41de@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20210807163749.18316-1-pali@kernel.org>

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. 
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.

>  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;

WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: "Pali Rohár" <pali@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>,
	"David S. Miller" <davem@davemloft.net>,
	Guillaume Nault <gnault@redhat.com>,
	linux-ppp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ppp: Add rtnl attribute IFLA_PPP_UNIT_ID for specifying ppp unit id
Date: Mon, 09 Aug 2021 19:25:46 +0000	[thread overview]
Message-ID: <20210809122546.758e41de@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20210807163749.18316-1-pali@kernel.org>

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. 
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.

>  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;

  reply	other threads:[~2021-08-09 19:25 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-07 16:37 [PATCH] ppp: Add rtnl attribute IFLA_PPP_UNIT_ID for specifying ppp unit id Pali Rohár
2021-08-07 16:37 ` Pali Rohár
2021-08-09 19:25 ` Jakub Kicinski [this message]
2021-08-09 19:25   ` Jakub Kicinski
2021-08-09 19:31   ` Pali Rohár
2021-08-09 19:31     ` Pali Rohár
2021-08-10 15:39     ` Guillaume Nault
2021-08-10 15:39       ` Guillaume Nault
2021-08-10 16:04       ` Pali Rohár
2021-08-10 16:04         ` Pali Rohár
2021-08-11 17:19         ` Guillaume Nault
2021-08-11 17:19           ` Guillaume Nault
2021-08-11 17:54           ` Pali Rohár
2021-08-11 17:54             ` Pali Rohár
2021-08-12  9:19             ` Guillaume Nault
2021-08-12  9:19               ` Guillaume Nault
2021-08-12 14:09               ` Pali Rohár
2021-08-12 14:09                 ` Pali Rohár
2021-08-12 19:12                 ` Guillaume Nault
2021-08-12 19:12                   ` Guillaume Nault
     [not found]       ` <BN0P223MB0327A247724B7AE211D2E84EA7F79@BN0P223MB0327.NAMP223.PROD.OUTLOOK.COM>
2021-08-10 17:16         ` Pali Rohár
2021-08-10 17:16           ` Pali Rohár
2021-08-10 18:11           ` James Carlson
2021-08-10 18:11             ` James Carlson
2021-08-11 17:38             ` Guillaume Nault
2021-08-11 17:38               ` Guillaume Nault
2021-08-11 18:04               ` Pali Rohár
2021-08-11 18:04                 ` Pali Rohár
2021-08-12  9:28                 ` Guillaume Nault
2021-08-12  9:28                   ` Guillaume Nault
2021-08-12 13:48                   ` Pali Rohár
2021-08-12 13:48                     ` Pali Rohár
2021-08-12 18:26                     ` Guillaume Nault
2021-08-12 18:26                       ` Guillaume Nault
2021-08-12 19:04                       ` Pali Rohár
2021-08-12 19:04                         ` Pali Rohár
2021-08-16 16:11                         ` Guillaume Nault
2021-08-16 16:11                           ` Guillaume Nault
2021-08-16 16:23                           ` Pali Rohár
2021-08-16 16:23                             ` Pali Rohár
2021-08-17 16:05                             ` Guillaume Nault
2021-08-17 16:05                               ` Guillaume Nault
2021-08-17 16:21                               ` Pali Rohár
2021-08-17 16:21                                 ` Pali Rohár
2022-07-09 12:09                                 ` Pali Rohár
2022-07-12 17:34                                   ` Guillaume Nault

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210809122546.758e41de@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=gnault@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ppp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pali@kernel.org \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.