Raw can socket close and disabling filters

Raw can socket close and disabling filters

[laurent vaudoit]

Hi all,
i'm using a 3.10 kernel with rtpatch applied, with raw can features.
Running some debug in the release function, i've seen something weird (for 
me).

In the function, we get the socket informations
and then we unregister the notifier.

After this, we test if the socket is bound and which is the ifindex.

But when the unregister function is called, there is a call to the notifier 
with the event NETDEV_UNREGISTER, who leads to clear the socket inforamtions 
like bound and ifindex.

So in the release function we can never enter in the code specific to a 
device.

Is there something i've missed?

Thanks in advance for your answer.
Regards
Laurent

Re: Raw can socket close and disabling filters

[Oliver Hartkopp]

Hi Laurent,

On 04/22/2016 01:39 PM, laurent vaudoit wrote:

> i'm using a 3.10 kernel with rtpatch applied, with raw can features.
> Running some debug in the release function, i've seen something weird (for
> me).
>
> In the function, we get the socket informations
> and then we unregister the notifier.
>
> After this, we test if the socket is bound and which is the ifindex.
>
> But when the unregister function is called, there is a call to the notifier
> with the event NETDEV_UNREGISTER, who leads to clear the socket inforamtions
> like bound and ifindex.
>
> So in the release function we can never enter in the code specific to a
> device.

The relevant removal is protected by lock_sock().

So when the notifier runs with NETDEV_UNREGISTER it will remove the 
filters & bind status.

In this case the block in raw_release() won't have anything to do to 
release the filters and the bind state, right?

Looks double work - but I wonder why we can rely on a notifier call when 
we call unregister_netdevice_notifier() ??

Regards,
Oliver

Re: Raw can socket close and disabling filters

[laurent vaudoit]

Hi Oliver,

On Sat, Apr 23, 2016 at 3:33 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Hi Laurent,
>
> On 04/22/2016 01:39 PM, laurent vaudoit wrote:
>
>> i'm using a 3.10 kernel with rtpatch applied, with raw can features.
>> Running some debug in the release function, i've seen something weird (for
>> me).
>>
>> In the function, we get the socket informations
>> and then we unregister the notifier.
>>
>> After this, we test if the socket is bound and which is the ifindex.
>>
>> But when the unregister function is called, there is a call to the
>> notifier
>> with the event NETDEV_UNREGISTER, who leads to clear the socket
>> inforamtions
>> like bound and ifindex.
>>
>> So in the release function we can never enter in the code specific to a
>> device.
>
>
> The relevant removal is protected by lock_sock().
>
> So when the notifier runs with NETDEV_UNREGISTER it will remove the filters
> & bind status.
>
> In this case the block in raw_release() won't have anything to do to release
> the filters and the bind state, right?

i agree with this. The fact is that on my setup i need to send an
information to another µc when releasing a socket on a device, and so
my specific code was done in the release function, after the
unregister.
This is how i've seen this behaviour.
>
> Looks double work - but I wonder why we can rely on a notifier call when we
> call unregister_netdevice_notifier() ??

For me, the disable_all_filter part of code in release can not be
called, as the ro->bound will allways be 0.
>
> Regards,
> Oliver
>

Regards
Laurent

Re: Raw can socket close and disabling filters

[Oliver Hartkopp]

On 04/25/2016 09:19 AM, laurent vaudoit wrote:

>> So when the notifier runs with NETDEV_UNREGISTER it will remove the filters
>> & bind status.
>>
>> In this case the block in raw_release() won't have anything to do to release
>> the filters and the bind state, right?
>
> i agree with this. The fact is that on my setup i need to send an
> information to another µc when releasing a socket on a device, and so
> my specific code was done in the release function, after the
> unregister.

I think using raw_release() as trigger is always a good choice.

> This is how i've seen this behaviour.

Interesting information. I didn't know about the notifier running at 
unregister_netdevice_notifier() time.

>> Looks double work - but I wonder why we can rely on a notifier call when we
>> call unregister_netdevice_notifier() ??
>
> For me, the disable_all_filter part of code in release can not be
> called, as the ro->bound will allways be 0.

The question remains whether we can really rely on the notifier call 
which is obviously invoked by unregister_netdevice_notifier() ...
I'll have to dig if this behaviour is intentional and documented.
Until then I feel better to leave it as-is.

Regards,
Oliver

Re: Raw can socket close and disabling filters

[laurent vaudoit]

On Mon, Apr 25, 2016 at 9:54 AM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> On 04/25/2016 09:19 AM, laurent vaudoit wrote:
>
>>> So when the notifier runs with NETDEV_UNREGISTER it will remove the
>>> filters
>>> & bind status.
>>>
>>> In this case the block in raw_release() won't have anything to do to
>>> release
>>> the filters and the bind state, right?
>>
>>
>> i agree with this. The fact is that on my setup i need to send an
>> information to another µc when releasing a socket on a device, and so
>> my specific code was done in the release function, after the
>> unregister.
>
>
> I think using raw_release() as trigger is always a good choice.
>
>> This is how i've seen this behaviour.
>
>
> Interesting information. I didn't know about the notifier running at
> unregister_netdevice_notifier() time.
>
>>> Looks double work - but I wonder why we can rely on a notifier call when
>>> we
>>> call unregister_netdevice_notifier() ??
>>
>>
>> For me, the disable_all_filter part of code in release can not be
>> called, as the ro->bound will allways be 0.
>
>
> The question remains whether we can really rely on the notifier call which
> is obviously invoked by unregister_netdevice_notifier() ...
> I'll have to dig if this behaviour is intentional and documented.
> Until then I feel better to leave it as-is.
>

Ok, i will adapt my specific code, as i know this is not a weird
behaviour due to rtpatch or to my specificity.
Thanks
> Regards,
> Oliver

Regards
Laurent