Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [net-next iproute2 PATCH v3 1/2] iplink: hsr: add support for
 creating PRP device similar to HSR
From:   Murali Karicheri <m-karicheri2@ti.com>
To:     <davem@davemloft.net>, <kuba@kernel.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-api@vger.kernel.org>,
        <nsekhar@ti.com>, <grygorii.strashko@ti.com>,
        <vinicius.gomes@intel.com>, <stephen@networkplumber.org>,
        <kuznet@ms2.inr.ac.ru>
References: <20200717152205.826-1-m-karicheri2@ti.com>
Message-ID: <e6ac459e-b81c-48ee-d82c-36a533e2aa29@ti.com>
Date:   Thu, 6 Aug 2020 12:04:31 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200717152205.826-1-m-karicheri2@ti.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Stephen, Alexey,

On 7/17/20 11:22 AM, Murali Karicheri wrote:
> This patch enhances the iplink command to add a proto parameters to
> create PRP device/interface similar to HSR. Both protocols are
> quite similar and requires a pair of Ethernet interfaces. So re-use
> the existing HSR iplink command to create PRP device/interface as
> well. Use proto parameter to differentiate the two protocols.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>   dependent on the series "[net-next PATCH v3 0/7] Add PRP driver"
>   include/uapi/linux/if_link.h | 12 +++++++++++-
>   ip/iplink_hsr.c              | 19 +++++++++++++++++--
>   2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index a8901a39a345..fa2e3f642deb 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -904,7 +904,14 @@ enum {
>   #define IFLA_IPOIB_MAX (__IFLA_IPOIB_MAX - 1)
>   
>   
> -/* HSR section */
> +/* HSR/PRP section, both uses same interface */
> +
> +/* Different redundancy protocols for hsr device */
> +enum {
> +	HSR_PROTOCOL_HSR,
> +	HSR_PROTOCOL_PRP,
> +	HSR_PROTOCOL_MAX,
> +};
>   
>   enum {
>   	IFLA_HSR_UNSPEC,
> @@ -914,6 +921,9 @@ enum {
>   	IFLA_HSR_SUPERVISION_ADDR,	/* Supervision frame multicast addr */
>   	IFLA_HSR_SEQ_NR,
>   	IFLA_HSR_VERSION,		/* HSR version */
> +	IFLA_HSR_PROTOCOL,		/* Indicate different protocol than
> +					 * HSR. For example PRP.
> +					 */
>   	__IFLA_HSR_MAX,
>   };
>   
> diff --git a/ip/iplink_hsr.c b/ip/iplink_hsr.c
> index 7d9167d4e6a3..6ea138a23cbc 100644
> --- a/ip/iplink_hsr.c
> +++ b/ip/iplink_hsr.c
> @@ -25,7 +25,7 @@ static void print_usage(FILE *f)
>   {
>   	fprintf(f,
>   		"Usage:\tip link add name NAME type hsr slave1 SLAVE1-IF slave2 SLAVE2-IF\n"
> -		"\t[ supervision ADDR-BYTE ] [version VERSION]\n"
> +		"\t[ supervision ADDR-BYTE ] [version VERSION] [proto PROTOCOL]\n"
>   		"\n"
>   		"NAME\n"
>   		"	name of new hsr device (e.g. hsr0)\n"
> @@ -35,7 +35,9 @@ static void print_usage(FILE *f)
>   		"	0-255; the last byte of the multicast address used for HSR supervision\n"
>   		"	frames (default = 0)\n"
>   		"VERSION\n"
> -		"	0,1; the protocol version to be used. (default = 0)\n");
> +		"	0,1; the protocol version to be used. (default = 0)\n"
> +		"PROTOCOL\n"
> +		"	0 - HSR, 1 - PRP. (default = 0 - HSR)\n");
>   }
>   
>   static void usage(void)
> @@ -49,6 +51,7 @@ static int hsr_parse_opt(struct link_util *lu, int argc, char **argv,
>   	int ifindex;
>   	unsigned char multicast_spec;
>   	unsigned char protocol_version;
> +	unsigned char protocol = HSR_PROTOCOL_HSR;
>   
>   	while (argc > 0) {
>   		if (matches(*argv, "supervision") == 0) {
> @@ -64,6 +67,13 @@ static int hsr_parse_opt(struct link_util *lu, int argc, char **argv,
>   				invarg("version is invalid", *argv);
>   			addattr_l(n, 1024, IFLA_HSR_VERSION,
>   				  &protocol_version, 1);
> +		} else if (matches(*argv, "proto") == 0) {
> +			NEXT_ARG();
> +			if (!(get_u8(&protocol, *argv, 0) == HSR_PROTOCOL_HSR ||
> +			      get_u8(&protocol, *argv, 0) == HSR_PROTOCOL_PRP))
> +				invarg("protocol is invalid", *argv);
> +			addattr_l(n, 1024, IFLA_HSR_PROTOCOL,
> +				  &protocol, 1);
>   		} else if (matches(*argv, "slave1") == 0) {
>   			NEXT_ARG();
>   			ifindex = ll_name_to_index(*argv);
> @@ -140,6 +150,11 @@ static void hsr_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>   					 RTA_PAYLOAD(tb[IFLA_HSR_SUPERVISION_ADDR]),
>   					 ARPHRD_VOID,
>   					 b1, sizeof(b1)));
> +	if (tb[IFLA_HSR_PROTOCOL])
> +		print_int(PRINT_ANY,
> +			  "proto",
> +			  "proto %d ",
> +			  rta_getattr_u8(tb[IFLA_HSR_PROTOCOL]));
>   }
>   
>   static void hsr_print_help(struct link_util *lu, int argc, char **argv,
> Sorry, I missed you in my email on this patch set as I didn't realize
that the maintainers are different than the netdev maintainers. My bad.
The PRP driver support in kernel is merged by Dave to net-next and this
iproute2 change has to go with it. So please review and apply this if it
looks good. The kernel part merged is at

https://www.spinics.net/lists/linux-api/msg42615.html

-- 
Murali Karicheri
Texas Instruments