| Message ID | 20210125104055.79882-1-socketcan@hartkopp.net (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [RESEND,iproute2,5.11] iplink_can: add Classical CAN frame LEN8_DLC support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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!
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
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.
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
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.
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
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
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
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) {
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(+)