* Re: [PATCH net 2/3] can: gs_usb: gs_can_open(): fix race dev->can.state condition
[not found] ` <20220921083609.419768-3-mkl@pengutronix.de>
@ 2022-09-22 8:04 ` Matthieu Baerts
2022-09-22 8:23 ` Marc Kleine-Budde
0 siblings, 1 reply; 4+ messages in thread
From: Matthieu Baerts @ 2022-09-22 8:04 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev; +Cc: davem, kuba, linux-can, kernel, MPTCP Upstream
[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]
Hello,
On 21/09/2022 10:36, Marc Kleine-Budde wrote:
> The dev->can.state is set to CAN_STATE_ERROR_ACTIVE, after the device
> has been started. On busy networks the CAN controller might receive
> CAN frame between and go into an error state before the dev->can.state
> is assigned.
>
> Assign dev->can.state before starting the controller to close the race
> window.
>
> Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
> Link: https://lore.kernel.org/all/20220920195216.232481-1-mkl@pengutronix.de
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
FYI, we got a small conflict when merging -net in net-next in the MPTCP
tree due to this patch applied in -net:
5440428b3da6 ("can: gs_usb: gs_can_open(): fix race dev->can.state
condition")
and this one from net-next:
45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support")
The conflict has been resolved on our side[1] and the resolution we
suggest is attached to this email.
Cheers,
Matt
[1] https://github.com/multipath-tcp/mptcp_net-next/commit/671f1521b564
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
[-- Attachment #2: 671f1521b5648e99fb55bbd0f114e4433551d411.patch --]
[-- Type: text/x-patch, Size: 1001 bytes --]
diff --cc drivers/net/can/usb/gs_usb.c
index cc363f1935ce,c1ff3c046d62..5e0d280b0cd3
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@@ -956,11 -823,8 +956,12 @@@ static int gs_can_open(struct net_devic
if (ctrlmode & CAN_CTRLMODE_3_SAMPLES)
flags |= GS_CAN_MODE_TRIPLE_SAMPLE;
+ /* if hardware supports timestamps, enable it */
+ if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
+ flags |= GS_CAN_MODE_HW_TIMESTAMP;
+
/* finally start device */
+ dev->can.state = CAN_STATE_ERROR_ACTIVE;
dm->mode = cpu_to_le32(GS_CAN_MODE_START);
dm->flags = cpu_to_le32(flags);
rc = usb_control_msg(interface_to_usbdev(dev->iface),
@@@ -977,12 -842,6 +979,10 @@@
kfree(dm);
+ /* start polling timestamp */
+ if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
+ gs_usb_timestamp_init(dev);
+
- dev->can.state = CAN_STATE_ERROR_ACTIVE;
-
parent->active_channels++;
if (!(dev->can.ctrlmode & CAN_CTRLMODE_LISTENONLY))
netif_start_queue(netdev);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net 2/3] can: gs_usb: gs_can_open(): fix race dev->can.state condition
2022-09-22 8:04 ` [PATCH net 2/3] can: gs_usb: gs_can_open(): fix race dev->can.state condition Matthieu Baerts
@ 2022-09-22 8:23 ` Marc Kleine-Budde
2022-09-22 20:05 ` Jakub Kicinski
0 siblings, 1 reply; 4+ messages in thread
From: Marc Kleine-Budde @ 2022-09-22 8:23 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: netdev, davem, kuba, linux-can, kernel, MPTCP Upstream
[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]
On 22.09.2022 10:04:55, Matthieu Baerts wrote:
> On 21/09/2022 10:36, Marc Kleine-Budde wrote:
> > The dev->can.state is set to CAN_STATE_ERROR_ACTIVE, after the device
> > has been started. On busy networks the CAN controller might receive
> > CAN frame between and go into an error state before the dev->can.state
> > is assigned.
> >
> > Assign dev->can.state before starting the controller to close the race
> > window.
> >
> > Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
> > Link: https://lore.kernel.org/all/20220920195216.232481-1-mkl@pengutronix.de
> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>
> FYI, we got a small conflict when merging -net in net-next in the MPTCP
> tree due to this patch applied in -net:
>
> 5440428b3da6 ("can: gs_usb: gs_can_open(): fix race dev->can.state
> condition")
>
> and this one from net-next:
>
> 45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support")
>
> The conflict has been resolved on our side[1] and the resolution we
> suggest is attached to this email.
That patch looks good to me.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net 2/3] can: gs_usb: gs_can_open(): fix race dev->can.state condition
2022-09-22 8:23 ` Marc Kleine-Budde
@ 2022-09-22 20:05 ` Jakub Kicinski
2022-09-23 9:24 ` Matthieu Baerts
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2022-09-22 20:05 UTC (permalink / raw)
To: Matthieu Baerts
Cc: Marc Kleine-Budde, netdev, davem, linux-can, kernel, MPTCP Upstream
On Thu, 22 Sep 2022 10:23:38 +0200 Marc Kleine-Budde wrote:
> On 22.09.2022 10:04:55, Matthieu Baerts wrote:
> > FYI, we got a small conflict when merging -net in net-next in the MPTCP
> > tree due to this patch applied in -net:
> >
> > 5440428b3da6 ("can: gs_usb: gs_can_open(): fix race dev->can.state
> > condition")
> >
> > and this one from net-next:
> >
> > 45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support")
> >
> > The conflict has been resolved on our side[1] and the resolution we
> > suggest is attached to this email.
Thanks for the resolution! If you happen to remember perhaps throw
"manual merge" into the subject. That's what I search my inbox for
when merging, it will allow us to be even more lazy :)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net 2/3] can: gs_usb: gs_can_open(): fix race dev->can.state condition
2022-09-22 20:05 ` Jakub Kicinski
@ 2022-09-23 9:24 ` Matthieu Baerts
0 siblings, 0 replies; 4+ messages in thread
From: Matthieu Baerts @ 2022-09-23 9:24 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Marc Kleine-Budde, netdev, davem, linux-can, kernel, MPTCP Upstream
Hi Jakub,
On 22/09/2022 22:05, Jakub Kicinski wrote:
> On Thu, 22 Sep 2022 10:23:38 +0200 Marc Kleine-Budde wrote:
>> On 22.09.2022 10:04:55, Matthieu Baerts wrote:
>>> FYI, we got a small conflict when merging -net in net-next in the MPTCP
>>> tree due to this patch applied in -net:
>>>
>>> 5440428b3da6 ("can: gs_usb: gs_can_open(): fix race dev->can.state
>>> condition")
>>>
>>> and this one from net-next:
>>>
>>> 45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support")
>>>
>>> The conflict has been resolved on our side[1] and the resolution we
>>> suggest is attached to this email.
>
> Thanks for the resolution! If you happen to remember perhaps throw
> "manual merge" into the subject. That's what I search my inbox for
> when merging, it will allow us to be even more lazy :)
I thought you were going to ask me to impersonate "Stephen Rothwell" :-P
Good to know, I can sure do that! (Or at least try to remember that next
time)
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-23 9:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220921083609.419768-1-mkl@pengutronix.de>
[not found] ` <20220921083609.419768-3-mkl@pengutronix.de>
2022-09-22 8:04 ` [PATCH net 2/3] can: gs_usb: gs_can_open(): fix race dev->can.state condition Matthieu Baerts
2022-09-22 8:23 ` Marc Kleine-Budde
2022-09-22 20:05 ` Jakub Kicinski
2022-09-23 9:24 ` Matthieu Baerts
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).