[PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[Oliver Hartkopp]

The len8_dlc element is filled by the CAN interface driver and used for CAN
frame creation by the CAN driver when the CAN_CTRLMODE_CC_LEN8_DLC flag is
supported by the driver and enabled via netlink configuration interface.

Add the command line support for cc-len8-dlc for Linux 5.11+

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 ip/iplink_can.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ip/iplink_can.c b/ip/iplink_can.c
index 735ab941..6a26f3ff 100644
--- a/ip/iplink_can.c
+++ b/ip/iplink_can.c
@@ -35,10 +35,11 @@ static void print_usage(FILE *f)
 		"\t[ one-shot { on | off } ]\n"
 		"\t[ berr-reporting { on | off } ]\n"
 		"\t[ fd { on | off } ]\n"
 		"\t[ fd-non-iso { on | off } ]\n"
 		"\t[ presume-ack { on | off } ]\n"
+		"\t[ cc-len8-dlc { on | off } ]\n"
 		"\n"
 		"\t[ restart-ms TIME-MS ]\n"
 		"\t[ restart ]\n"
 		"\n"
 		"\t[ termination { 0..65535 } ]\n"
@@ -101,10 +102,11 @@ static void print_ctrlmode(FILE *f, __u32 cm)
 	_PF(CAN_CTRLMODE_ONE_SHOT, "ONE-SHOT");
 	_PF(CAN_CTRLMODE_BERR_REPORTING, "BERR-REPORTING");
 	_PF(CAN_CTRLMODE_FD, "FD");
 	_PF(CAN_CTRLMODE_FD_NON_ISO, "FD-NON-ISO");
 	_PF(CAN_CTRLMODE_PRESUME_ACK, "PRESUME-ACK");
+	_PF(CAN_CTRLMODE_CC_LEN8_DLC, "CC-LEN8-DLC");
 #undef _PF
 	if (cm)
 		print_hex(PRINT_ANY, NULL, "%x", cm);
 	close_json_array(PRINT_ANY, "> ");
 }
@@ -209,10 +211,14 @@ static int can_parse_opt(struct link_util *lu, int argc, char **argv,
 				     CAN_CTRLMODE_FD_NON_ISO);
 		} else if (matches(*argv, "presume-ack") == 0) {
 			NEXT_ARG();
 			set_ctrlmode("presume-ack", *argv, &cm,
 				     CAN_CTRLMODE_PRESUME_ACK);
+		} else if (matches(*argv, "cc-len8-dlc") == 0) {
+			NEXT_ARG();
+			set_ctrlmode("cc-len8-dlc", *argv, &cm,
+				     CAN_CTRLMODE_CC_LEN8_DLC);
 		} else if (matches(*argv, "restart") == 0) {
 			__u32 val = 1;
 
 			addattr32(n, 1024, IFLA_CAN_RESTART, val);
 		} else if (matches(*argv, "restart-ms") == 0) {
-- 
2.29.2

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[Vincent MAILHOL]

On Mon. 25 Jan 2021 at 19:40, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> The len8_dlc element is filled by the CAN interface driver and used for CAN
> frame creation by the CAN driver when the CAN_CTRLMODE_CC_LEN8_DLC flag is
> supported by the driver and enabled via netlink configuration interface.
>
> Add the command line support for cc-len8-dlc for Linux 5.11+
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
>  ip/iplink_can.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/ip/iplink_can.c b/ip/iplink_can.c
> index 735ab941..6a26f3ff 100644
> --- a/ip/iplink_can.c
> +++ b/ip/iplink_can.c
> @@ -35,10 +35,11 @@ static void print_usage(FILE *f)
>                 "\t[ one-shot { on | off } ]\n"
>                 "\t[ berr-reporting { on | off } ]\n"
>                 "\t[ fd { on | off } ]\n"
>                 "\t[ fd-non-iso { on | off } ]\n"
>                 "\t[ presume-ack { on | off } ]\n"
> +               "\t[ cc-len8-dlc { on | off } ]\n"
>                 "\n"
>                 "\t[ restart-ms TIME-MS ]\n"
>                 "\t[ restart ]\n"
>                 "\n"
>                 "\t[ termination { 0..65535 } ]\n"
> @@ -101,10 +102,11 @@ static void print_ctrlmode(FILE *f, __u32 cm)
>         _PF(CAN_CTRLMODE_ONE_SHOT, "ONE-SHOT");
>         _PF(CAN_CTRLMODE_BERR_REPORTING, "BERR-REPORTING");
>         _PF(CAN_CTRLMODE_FD, "FD");
>         _PF(CAN_CTRLMODE_FD_NON_ISO, "FD-NON-ISO");
>         _PF(CAN_CTRLMODE_PRESUME_ACK, "PRESUME-ACK");
> +       _PF(CAN_CTRLMODE_CC_LEN8_DLC, "CC-LEN8-DLC");
>  #undef _PF
>         if (cm)
>                 print_hex(PRINT_ANY, NULL, "%x", cm);
>         close_json_array(PRINT_ANY, "> ");
>  }
> @@ -209,10 +211,14 @@ static int can_parse_opt(struct link_util *lu, int argc, char **argv,
>                                      CAN_CTRLMODE_FD_NON_ISO);
>                 } else if (matches(*argv, "presume-ack") == 0) {
>                         NEXT_ARG();
>                         set_ctrlmode("presume-ack", *argv, &cm,
>                                      CAN_CTRLMODE_PRESUME_ACK);
> +               } else if (matches(*argv, "cc-len8-dlc") == 0) {
> +                       NEXT_ARG();
> +                       set_ctrlmode("cc-len8-dlc", *argv, &cm,
> +                                    CAN_CTRLMODE_CC_LEN8_DLC);
>                 } else if (matches(*argv, "restart") == 0) {
>                         __u32 val = 1;
>
>                         addattr32(n, 1024, IFLA_CAN_RESTART, val);
>                 } else if (matches(*argv, "restart-ms") == 0) {
> --
> 2.29.2

Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

Reviewed and tested the patch, everything is OK for me.
Thanks Oliver!

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[Oliver Hartkopp]

Hello David,

On 29.01.21 16:50, David Ahern wrote:
> On 1/25/21 3:40 AM, Oliver Hartkopp wrote:
>> The len8_dlc element is filled by the CAN interface driver and used for CAN
>> frame creation by the CAN driver when the CAN_CTRLMODE_CC_LEN8_DLC flag is
>> supported by the driver and enabled via netlink configuration interface.
>>
>> Add the command line support for cc-len8-dlc for Linux 5.11+
>>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>>   ip/iplink_can.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
> 
> 
> applied to iproute2-next
> 

Are you sure this patch is correctly assigned to iproute2-next?

IMO it has to be applied to iproute2 as the functionality is already in 
v5.11 which is in rc6 right now.

Regards,
Oliver

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[David Ahern]

On 2/2/21 3:48 AM, Oliver Hartkopp wrote:
> 
> Are you sure this patch is correctly assigned to iproute2-next?
> 
> IMO it has to be applied to iproute2 as the functionality is already in
> v5.11 which is in rc6 right now.
> 

new features land in iproute2-next just as they do for the kernel with
net-next.

Patches adding support for kernel features should be sent in the same
development window if you want the iproute2 support to match kernel version.

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[Oliver Hartkopp]

On 02.02.21 16:35, David Ahern wrote:
> On 2/2/21 3:48 AM, Oliver Hartkopp wrote:
>>
>> Are you sure this patch is correctly assigned to iproute2-next?
>>
>> IMO it has to be applied to iproute2 as the functionality is already in
>> v5.11 which is in rc6 right now.
>>
> 
> new features land in iproute2-next just as they do for the kernel with
> net-next.
> 
> Patches adding support for kernel features should be sent in the same
> development window if you want the iproute2 support to match kernel version.
> 

Oh, I followed the commits from iproute2 until the new include files 
from (in this case) 5.11 pre rc1 had been updated (on 2020-12-24):

https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=2953235e61eb672bbdd2de84eb5b91c388f9a9b5

I thought the uapi updates in iproute2 are *always* pulled from the 
kernel and not from iprout2-next which was new to me.

Do you expect patches for iproute2-next when the relevant changes become 
available in linux-next then?

Even though I did not know about iproute2-next the patch is needed for 
the 5.11 kernel (as written in the subject).

Regards,
Oliver

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[David Ahern]

On 2/2/21 10:30 AM, Oliver Hartkopp wrote:
> 
> 
> On 02.02.21 16:35, David Ahern wrote:
>> On 2/2/21 3:48 AM, Oliver Hartkopp wrote:
>>>
>>> Are you sure this patch is correctly assigned to iproute2-next?
>>>
>>> IMO it has to be applied to iproute2 as the functionality is already in
>>> v5.11 which is in rc6 right now.
>>>
>>
>> new features land in iproute2-next just as they do for the kernel with
>> net-next.
>>
>> Patches adding support for kernel features should be sent in the same
>> development window if you want the iproute2 support to match kernel
>> version.
>>
> 
> Oh, I followed the commits from iproute2 until the new include files
> from (in this case) 5.11 pre rc1 had been updated (on 2020-12-24):
> 
> https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=2953235e61eb672bbdd2de84eb5b91c388f9a9b5
> 
> 
> I thought the uapi updates in iproute2 are *always* pulled from the
> kernel and not from iprout2-next which was new to me.

I sync kernel headers for iproute2-next with net-next, not linux-next.

> 
> Do you expect patches for iproute2-next when the relevant changes become
> available in linux-next then?
> 
> Even though I did not know about iproute2-next the patch is needed for
> the 5.11 kernel (as written in the subject).
> 


From a cursory look it appears CAN commits do not go through the netdev
tree yet the code is under net/can and the admin tool is through
iproute2 and netdevs. Why is that? If features patches flowed through
net-next, we would not have this problem.

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[Oliver Hartkopp]

On 03.02.21 16:47, David Ahern wrote:
> On 2/2/21 10:30 AM, Oliver Hartkopp wrote:
>>
>>
>> On 02.02.21 16:35, David Ahern wrote:
>>> On 2/2/21 3:48 AM, Oliver Hartkopp wrote:
>>>>
>>>> Are you sure this patch is correctly assigned to iproute2-next?
>>>>
>>>> IMO it has to be applied to iproute2 as the functionality is already in
>>>> v5.11 which is in rc6 right now.
>>>>
>>>
>>> new features land in iproute2-next just as they do for the kernel with
>>> net-next.
>>>
>>> Patches adding support for kernel features should be sent in the same
>>> development window if you want the iproute2 support to match kernel
>>> version.
>>>
>>
>> Oh, I followed the commits from iproute2 until the new include files
>> from (in this case) 5.11 pre rc1 had been updated (on 2020-12-24):
>>
>> https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=2953235e61eb672bbdd2de84eb5b91c388f9a9b5
>>
>>
>> I thought the uapi updates in iproute2 are *always* pulled from the
>> kernel and not from iprout2-next which was new to me.
> 
> I sync kernel headers for iproute2-next with net-next, not linux-next.

Ok. Got it.

>>
>> Do you expect patches for iproute2-next when the relevant changes become
>> available in linux-next then?
>>
>> Even though I did not know about iproute2-next the patch is needed for
>> the 5.11 kernel (as written in the subject).
>>
> 
> 
>  From a cursory look it appears CAN commits do not go through the netdev
> tree yet the code is under net/can and the admin tool is through
> iproute2 and netdevs. Why is that? If features patches flowed through
> net-next, we would not have this problem.

CAN commits go through linux-can-next -> net-next. Same for linux-can -> 
net.

The len8_dlc patches also went through linux-can-next -> net-next -> net 
-> linux

iproute2 provides the configuration interface for CAN drivers under 
driver/net/can only.

My only fault was, that I did not send the patch for iproute2-next at 
the time when the len8_dlc patches were in net-next, right?

I was just not aware of iproute2-next.

The former patches I posted for iproute2 were always applied by Stephen 
to the iproute2 tree directly.

Regards,
Oliver

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[David Ahern]

On 2/3/21 12:04 PM, Oliver Hartkopp wrote:
> My only fault was, that I did not send the patch for iproute2-next at
> the time when the len8_dlc patches were in net-next, right?

yes

Re: [PATCH RESEND iproute2 5.11] iplink_can: add Classical CAN frame LEN8_DLC support

[Oliver Hartkopp]

On 04.02.21 04:15, David Ahern wrote:
> On 2/3/21 12:04 PM, Oliver Hartkopp wrote:
>> My only fault was, that I did not send the patch for iproute2-next at
>> the time when the len8_dlc patches were in net-next, right?
> 
> yes
> 

Now that I know about iproute2-next, I will do better next time.

Can you please apply this simple patch intended for Linux 5.11 to the 
iproute2 tree this time?

Thanks,
Oliver