On 5/11/22 15:05, Marc Kleine-Budde wrote:
> On 11.05.2022 14:38:32, Oliver Hartkopp wrote:
>> I'm a bit unsure why we should not stick with the simple skb->sk
>> handling?
>
> Another use where skb->sk breaks is sending CAN frames with SO_TXTIME
> with the sched_etf.
>
> I have a patched version of cangen that uses SO_TXTIME. It attaches a
> time to transmit to a CAN frame when sending it. If you send 10 frames,
> each 100ms after the other and then exit the program, the first CAN
> frames show up as TX'ed while the others (after closing the socket) show
> up as RX'ed CAN frames in candump.
Hm, this could be an argument for the origin flag.
But I'm more scared about your described behaviour. What happens if the
socket is still open?
There obviously must be some instance removing the sk reference, right??
Regards,
Oliver
On 11.05.2022 15:24:00, Oliver Hartkopp wrote:
> > Another use where skb->sk breaks is sending CAN frames with SO_TXTIME
> > with the sched_etf.
> >
> > I have a patched version of cangen that uses SO_TXTIME. It attaches a
> > time to transmit to a CAN frame when sending it. If you send 10 frames,
> > each 100ms after the other and then exit the program, the first CAN
> > frames show up as TX'ed while the others (after closing the socket) show
> > up as RX'ed CAN frames in candump.
>
> Hm, this could be an argument for the origin flag.
>
> But I'm more scared about your described behaviour. What happens if
> the socket is still open?
SO_TXTIME makes an existing race window really, really, really wide.
The race window is between sendmsg() returning to user space and
can_put_echo_skb() -> can_create_echo_skb() -> can_skb_set_owner(). In
can_skb_set_owner() a reference to the socket is taken, if the socket is
not closed:
| https://elixir.bootlin.com/linux/v5.17.6/source/include/linux/can/skb.h#L75
If the socket closes _after_ sendmsg(), but _before_ the driver calls
can_put_echo_skb() the CAN frame will have no socket reference and show
up as RX'ed.
> There obviously must be some instance removing the sk reference, right??
No. That's all fine. We fixes that in:
| https://lore.kernel.org/all/[email protected]/
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 |