Hi,
In addition to Marc’s comment, I also have security concerns.
On Fri. 22 Jan 2021 at 15:22, Su Yanjun wrote:
> When i debug mcp2518fd, some method to track registers is
> needed. This easy debug interface will be ok.
>
> For example,
> read a register at 0xe00:
> echo 0xe00 > can_get_reg
>
CC of each of the patch of the series
(sorry for the mess in v1...)
Vincent Mailhol (7):
can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ
context
can: dev: fix type of get_can_dlc() and get_canfd_dlc() macros
can: dev: add a helper function to get the correct length of
that only CAN devices are affected by this issue).
[1] https://patchwork.ozlabs.org/patch/835236/
Signed-off-by: Vincent Mailhol
---
Changes in v3: None
Changes in v2:
- Minor changes of link format in the changelog.
---
drivers/net/can/dev.c | 6 +-
1 file changed, 5 insertions(+),
The macros get_can_dlc() and get_canfd_dlc() are not visible in
userland. As such, type u8 should be preferred over type __u8.
Reference: https://lkml.org/lkml/2020/10/1/708
Signed-off-by: Vincent Mailhol
---
include/linux/can/dev.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions
classical CAN
frames: this is incorrect!
This patch introduces function get_can_len() which can be used in
remediation. The function takes the SKB as an input in order to be
able to determine if the frame is classical or FD.
Signed-off-by: Vincent Mailhol
---
Changes in v3:
- Make get_can_len()
The length of Remote Transmission Request (RTR) frames is always 0
bytes. The DLC represents the requested length, not the actual length
of the RTR. But __can_get_echo_skb() returns the DLC value regardless.
Apply get_can_len() function to retrieve the correct length.
Signed-off-by: Vincent
in the CAN ISO standard. Device drivers for
CAN might need those.
Please refer to ISO 11898-1:2015, section 11.3.1.1 "Bit time" for
additional information.
Signed-off-by: Vincent Mailhol
---
Changes in v3: None
Changes in v2: None
---
drivers/net/can/dev.c | 13 ++---
include
-by: Vincent Mailhol
---
Changes in v3: None
Changes in v2:
- Added dmesg and lsusb -v information and rephrased the comment.
---
drivers/usb/class/cdc-acm.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index
Take care to put everyone in CC of each of the patch of the series
(sorry for the mess in v1...)
Vincent Mailhol (6):
can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ
context
can: dev: add a helper function to get the correct length of Classical
frames
that only CAN devices are affected by this issue).
[1] https://patchwork.ozlabs.org/patch/835236/
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 68834a2853c9..e2
classical CAN
frames: this is incorrect!
This patch introduces function get_can_len() which can be used in
remediation. The function takes the SKB as an input in order to be
able to determine if the frame is classical or FD.
Signed-off-by: Vincent Mailhol
---
include/linux/can/dev.h | 23 +++
The length of Remote Transmission Request (RTR) frames is always 0
bytes. The DLC represents the requested length, not the actual length
of the RTR. But __can_get_echo_skb() returns the DLC value regardless.
Apply get_can_len() function to retrieve the correct length.
Signed-off-by: Vincent
in the CAN ISO standard. Device drivers for
CAN might need those.
Please refer to ISO 11898-1:2015, section 11.3.1.1 "Bit time" for
additional information.
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 13 ++---
include/linux/can/dev.h | 15 +++
2 fil
-by: Vincent Mailhol
---
drivers/usb/class/cdc-acm.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 7f6f3ab5b8a6..ed9355094e8c 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1906,6
> > +static inline int get_can_len(struct sk_buff *skb)
>
> make this return an u8
> make the skb const
>
> > +{
> > + struct canfd_frame *cf =3D (struct canfd_frame *)skb->data;
>
> const
>
> > +
> > + if (can_is_canfd_skb(skb))
> > + return min_t(__u8, cf->len, CANFD_MAX_DLEN);
> > + num_element =
> > + es58x_msg_num_element(es58x_dev->dev,
> > + bulk_rx_loopback_msg->rx_loopback_msg,
> > + msg_len);
> > + if (unlikely(num_element <= 0))
> > + return num_element;
>
> Meta-comment on your use of
n in PATCH 7/7 and rephrased the comment.
- Take care to put everyone in CC of each of the patch of the series
(sorry for the mess in v1...)
Vincent Mailhol (4):
can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ
context
can: dev: add a helper function to get the corr
that only CAN devices are affected by this issue).
[1] https://patchwork.ozlabs.org/patch/835236/
Signed-off-by: Vincent Mailhol
---
Changes in v3 and v4: None
Changes in v2:
- Minor changes of link format in the changelog.
---
drivers/net/can/dev.c | 6 +-
1 file changed, 5 insertions(+),
classical CAN
frames: this is incorrect!
This patch introduces function get_can_len() which can be used in
remediation. The function takes the SKB as an input in order to be
able to determine if the frame is classical or FD.
Signed-off-by: Vincent Mailhol
---
Changes in v4: None
Changes in v3:
The length of Remote Transmission Request (RTR) frames is always 0
bytes. The DLC represents the requested length, not the actual length
of the RTR. But __can_get_echo_skb() returns the DLC value regardless.
Apply get_can_len() function to retrieve the correct length.
Signed-off-by: Vincent
gt; You can provide an outer macro that does the sizeof() and then calls the a
> normal (static inline) function to do the actual work. Applied to the next 3
> macros.
OK. Will be fixed in v4.
> > +#define ES58X_SIZEOF_URB_CMD(es58x_urb_cmd_type, msg_field)
> > \
> > + (offsetof(es58x_urb_cmd_type, raw_msg) \
> > + + sizeof_field(es58x_urb_cmd_type, msg_field) \
> > + + sizeof_field(es58x_urb_cmd_type, \
> > + reserved_for_crc16_do_not_use))
>
> static inline?
Sorry but this one can not be converted into a static inline: the
first argument is a type (that will become the first argument of
offsetof() and sizeof_field()).
One more time, thank you for your time and your review!
Yours sincerely,
Vincent Mailhol
eem to have been ignored.
FYI, I used the below command to generate the patch.
git format-patch --base=bff6f1db91e330d7fba56f815cdbc412c75fe163 -v6
-o patch/v6 HEAD~1
Yours sincerely,
Vincent Mailhol
> url:
> https://github.com/0day-ci/linux/commits/Vincent-Mailhol/can-usb-etas_es58
On Mon. 16 Nov 2020 at 03:55, Marc Kleine-Budde wrote:
> On 11/14/20 4:22 PM, Vincent Mailhol wrote:
> > This driver supports the ES581.4, ES582.1 and ES584.1 interfaces from
> > ETAS GmbH (https://www.etas.com/en/products/es58x.php).
> >
> > Co-developed-by: Arunachala
that relies on the error queue such as
skb_tx_timestamp().
SCM_CAN_RAW_ERRQUEUE is defined as the type for the CAN raw error
queue. SCM stands for "Socket control messages". The name is inspired
from SCM_J1939_ERRQUEUE of include/uapi/linux/can/j1939.h.
Signed-off-by: Vincent Mailhol
---
in
assumption behind is that only CAN devices
are affected by this issue).
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 68834a2853c9..e291fda395a0 100644
--- a/drive
The length of Remote Transmission Request (RTR) frames is always 0
bytes. The DLC represents the requested length, not the actual length
of the RTR. But __can_get_echo_skb() returns the DLC value regardless.
Apply get_can_len() function to retrieve the correct length.
Signed-off-by: Vincent
in the CAN ISO standard. Device drivers for
CAN might need those.
Please refer to ISO 11898-1:2015, section 11.3.1.1 "Bit time" for
additional information.
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 13 ++---
include/linux/can/dev.h | 15 +++
2 fil
assumption behind is that only CAN devices
are affected by this issue).
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 68834a2853c9..e291fda395a0 100644
--- a/drive
classical CAN
frames: this is incorrect!
This patch introduces function get_can_len() which can be used in
remediation. The function takes the SKB as an input in order to be
able to determine if the frame is classical or FD.
Signed-off-by: Vincent Mailhol
---
include/linux/can/dev.h | 23 +++
The ES58X devices are incorrectly recognized as USB Modem (CDC ACM),
preventing the etas-es58x module to load.
Thus, these have been added
to the ignore list in drivers/usb/class/cdc-acm.c
Signed-off-by: Vincent Mailhol
---
drivers/usb/class/cdc-acm.c | 11 +++
1 file changed, 11
assumption behind is that only CAN devices
are affected by this issue).
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 68834a2853c9..e291fda395a0 100644
--- a/drive
r comments.
Vincent Mailhol (6):
can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ
context
can: dev: add a helper function to get the correct length of Classical
frames
can: dev: __can_get_echo_skb(): fix the return length
can: dev: add a helper function to
classical CAN
frames: this is incorrect!
This patch introduces function get_can_len() which can be used in
remediation. The function takes the SKB as an input in order to be
able to determine if the frame is classical or FD.
Signed-off-by: Vincent Mailhol
---
include/linux/can/dev.h | 23 +++
The length of Remote Transmission Request (RTR) frames is always 0
bytes. The DLC represents the requested length, not the actual length
of the RTR. But __can_get_echo_skb() returns the DLC value regardless.
Apply get_can_len() function to retrieve the correct length.
Signed-off-by: Vincent
in the CAN ISO standard. Device drivers for
CAN might need those.
Please refer to ISO 11898-1:2015, section 11.3.1.1 "Bit time" for
additional information.
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 13 ++---
include/linux/can/dev.h | 15 +++
2 fil
The ES58X devices are incorrectly recognized as USB Modem (CDC ACM),
preventing the etas-es58x module to load.
Thus, these have been added
to the ignore list in drivers/usb/class/cdc-acm.c
Signed-off-by: Vincent Mailhol
---
drivers/usb/class/cdc-acm.c | 11 +++
1 file changed, 11
> > Did you mean to send this twice?
Sorry for that, I screwed things up a first time when sending the
patches: only included the CAN mailing list
(linux-...@vger.kernel.org) but ommitted linux-kernel@vger.kernel.org
in the cover letter. As a result, it broke the chain reply on lkml.org
so I
is
exactly 8 and the len8_dlc value of the previous iteration will be
sent again.
This can be observed by running 'cangen -8 can0'. Once a frame of DLC
in the range 9..15 is generated, no more frames of DLC 8 show out in
the log.
Signed-off-by: Vincent Mailhol
---
cangen.c | 2 +-
1 file
On Tue. 12 Jan 2021 at 02:11, Richard Cochran wrote:
>
> On Sun, Jan 10, 2021 at 09:49:03PM +0900, Vincent Mailhol wrote:
> > * The hardware rx timestamp of a local loopback message is the
> > hardware tx timestamp. This means that there are no needs to
On Tue. 12 Jan 2021 at 11:14, Richard Cochran wrote:
>
> On Tue, Jan 12, 2021 at 09:00:33AM +0900, Vincent MAILHOL wrote:
> > Out of curiosity, which programs do you use? I guess wireshark
> > but please let me know if you use any other programs (I just use
> > to write
On Tue. 12 Jan 2021 at 16:58, Marc Kleine-Budde wrote:
>
> On 1/12/21 1:00 AM, Vincent MAILHOL wrote:
> [...]
>
> > Mark: do you want me to send a v4 of that patch with above
> > comment removed or can you directly do the change in your testing
> > branch?
>
>
between the kernel tx
software timestamp and the userland tx software timestamp).
Vincent Mailhol (1):
can: dev: add software tx timestamps
drivers/net/can/dev.c | 2 ++
1 file changed, 2 insertions(+)
--
2.26.2
for the error queue in CAN RAW sockets (which is needed for tx
timestamps) was introduced in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eb88531bdbfaafb827192d1fc6c5a3fcc4fadd96
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 2 ++
1 file changed, 2
Hello Jeroen,
On Sun. 10 Jan 2021 at 20:29, Jeroen Hofstee wrote:
>
> Hello Vincent,
>
> On 1/10/21 11:35 AM, Vincent Mailhol wrote:
> > Call skb_tx_timestamp() within can_put_echo_skb() so that a software
> > tx timestamp gets attached on the skb.
> >
> [..]
>
between the kernel tx
software timestamp and the userland tx software timestamp).
v2 reflects the comments that Jeroen made in
https://lkml.org/lkml/2021/1/10/54
Vincent Mailhol (1):
can: dev: add software tx timestamps
drivers/net/can/dev.c | 2 ++
1 file changed, 2 insertions(+)
--
2.26.2
/lkml/2021/1/10/54
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3486704c8a95..3904e0874543 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -484,6 +484,8 @@ int
between the kernel tx
software timestamp and the userland tx software timestamp).
v2 was a mistake, please ignore it (fogot to do git add, changes were
not reflected...)
v3 reflects the comments that Jeroen made in
https://lkml.org/lkml/2021/1/10/54
Vincent Mailhol (1):
can: dev: add software
/lkml/2021/1/10/54
Signed-off-by: Vincent Mailhol
---
drivers/net/can/dev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3486704c8a95..850759c7677f 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -481,6 +481,7 @@ int
Hi Oliver and Rong,
This is an interesting and quite surprising issue!
On Tue. 23 mars 2021 at 11:54, Rong Chen wrote:
> On 3/23/21 12:24 AM, Oliver Hartkopp wrote:
> > Hi Rong,
> >
> > On 22.03.21 09:52, Rong Chen wrote:
> >
> >> On 3/21/21 10:19 PM, Oliver Hartkopp wrote:
> >>> Two reminders
__must_be_array, offsetof, sizeof_field and __stringify are all
preprocessor macros and do not evaluate their arguments. As such, it
is safe not to warn when arguments are being reused in those four
sub-expressions.
Exclude those so that they can pass checkpatch.
Signed-off-by: Vincent Mailhol
in that branch to compile.
Crossing fingers, hoping that we are now close to a release.
Thank you in advance for your review and for your time!
Yours sincerely,
Vincent
Vincent Mailhol (1):
can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces
drivers/net/can/usb/Kconfig
list:
https://lore.kernel.org/linux-can/20210309125708.ei75tr5vp2san...@pengutronix.de/
Thank you for your comments.
Yours sincerely,
Vincent
Vincent Mailhol (1):
dql: add dql_set_min_limit()
include/linux/dynamic_queue_limits.h | 3 +++
lib/dynamic_queue_limits.c | 8
2 files
Add a function to set the dynamic queue limit minimum value.
This function is to be used by network drivers which are able to
prove, at least through empirical tests, that they reach better
performances with a specific predefined dql.min_limit value.
Signed-off-by: Vincent Mailhol
---
include
On Wed. 10 Mar 2021 at 03:11, Vincent MAILHOL
wrote:
>
> On Wed. 10 Mar 2021 at 02:27, Jimmy Assarsson
> wrote:
> >
> > Hi Vincent,
> >
> > On 2021-03-09 13:09, Vincent Mailhol wrote:
> > > This driver supports the ES581.4, ES582.1 and ES58
On Wed. 10 Mar 2021 at 00:23, Vincent Mailhol
wrote:
>
> Add a function to set the dynamic queue limit minimum value.
>
> This function is to be used by network drivers which are able to
> prove, at least through empirical tests, that they reach better
> performances with a s
On Wed. 10 Mar 2021 at 02:27, Jimmy Assarsson wrote:
>
> Hi Vincent,
>
> On 2021-03-09 13:09, Vincent Mailhol wrote:
> > This driver supports the ES581.4, ES582.1 and ES584.1 interfaces from
> > ETAS GmbH (https://www.etas.com/en/products/es58x.php).
> ...
> >
Hi Dave,
Thanks for the comprehensive comments!
On Wed. 10 Mar 2021 at 04:44, Dave Taht wrote:
>
> I note that "proof" is very much in the developer's opinion and
> limited testing base.
>
> Actual operational experience, as in a real deployment, with other
> applications,
> heavy context
performances with a specific predefined
dql.min_limit value with no noticeable latency impact.
Signed-off-by: Vincent Mailhol
---
include/linux/netdevice.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ddf4cfc12615
o RFC v1:
https://lore.kernel.org/linux-can/20210309152354.95309-1-mailhol.vinc...@wanadoo.fr/T/#t
Vincent Mailhol (1):
dql: add dql_set_min_limit()
include/linux/netdevice.h | 17 +
1 file changed, 17 insertions(+)
--
2.26.2
__must_be_array, offsetof, sizeof_field and __stringify are all
preprocessor macros and do not evaluate their arguments. As such, it
is safe not to warn when arguments are being reused in those four
sub-expressions.
Exclude those so that they can pass checkpatch.
Signed-off-by: Vincent Mailhol
switching, virtualization...),
that they constantly reach better performances with a specific
predefined dql.min_limit value with no noticeable latency impact.
Signed-off-by: Vincent Mailhol
---
include/linux/netdevice.h | 18 ++
1 file changed, 18 insertions(+)
diff --git
://lore.kernel.org/linux-can/20210309153547.q7zspf46k6ter...@pengutronix.de/
Link to RFC v1:
https://lore.kernel.org/linux-can/20210309152354.95309-1-mailhol.vinc...@wanadoo.fr/T/#t
Vincent Mailhol (1):
netdev: add netdev_queue_set_dql_min_limit()
include/linux/netdevice.h | 18
__must_be_array, offsetof, sizeof_field and __stringify are all
preprocessor macros and do not evaluate their arguments. As such, it
is safe not to warn when arguments are being reused in those four
sub-expressions.
Exclude those so that they can pass checkpatch.
Signed-off-by: Vincent Mailhol
TAS ES58X CAN
> > USB interfaces")
> > Signed-off-by: Colin Ian King
Acked-by: Vincent Mailhol
Thanks Colin!
Hi Marc,
On Mon. 12 Apr 2021 at 18:20, Marc Kleine-Budde wrote:
> On 10.04.2021 18:59:45, Vincent Mailhol wrote:
> > Here comes the 15th iteration of the patch. This new version addresses
> > the comments received from Marc (thanks again for the review!) and
> > simpli
-by: Vincent Mailhol
---
drivers/net/can/usb/Kconfig | 10 +
drivers/net/can/usb/Makefile|1 +
drivers/net/can/usb/etas_es58x/Makefile |3 +
drivers/net/can/usb/etas_es58x/es58x_core.c | 2281 +++
drivers/net/can/usb/etas_es58x/es58x_core.h
with -W1).
v1 (2020-09-27):
- First release
Vincent Mailhol (3):
can: etas_es58x: add core support for ETAS ES58X CAN USB interfaces
can: etas_es58x: add support for ETAS ES581.4 CAN USB interface
can: etas_es58x: add support for the ETAS ES58X_FD CAN USB interfaces
driv
This patch add support for the ES582.1 and ES584.1 interfaces from
ETAS GmbH (https://www.etas.com/en/products/es58x.php).
Co-developed-by: Arunachalam Santhanam
Signed-off-by: Arunachalam Santhanam
Signed-off-by: Vincent Mailhol
---
drivers/net/can/usb/etas_es58x/Makefile | 2
This patch adds support for the ES581.4 interface from ETAS
GmbH (https://www.etas.com/en/products/es58x.php).
Co-developed-by: Arunachalam Santhanam
Signed-off-by: Arunachalam Santhanam
Signed-off-by: Vincent Mailhol
---
drivers/net/can/usb/etas_es58x/Makefile | 2 +-
drivers/net/can
LC (which is the
length of any CAN error frames) solves this NULL pointer dereference.
Fixes: 8537257874e9 ("can: etas_es58x: add core support for ETAS ES58X CAN USB
interfaces")
Reported-by: Arunachalam Santhanam
Signed-off-by: Vincent Mailhol
---
Hi Marc,
I am really sorry, but I was just notifi
70 matches
Mail list logo
