Re: [PATCH net-next v02 1/2] af_packet: allow fanout_add when socket is not RUNNING

[Gur Stavi]

>> @@ -1846,21 +1846,21 @@ static int fanout_add(struct sock *sk, struct fanout_args *args)
>>  	err = -EINVAL;
>>
>>  	spin_lock(&po->bind_lock);
>> -	if (packet_sock_flag(po, PACKET_SOCK_RUNNING) &&
>> -	    match->type == type &&
>> +	if (match->type == type &&
>>  	    match->prot_hook.type == po->prot_hook.type &&
>>  	    match->prot_hook.dev == po->prot_hook.dev) {
>
> Remaining unaddressed issue is that the socket can now be added
> before being bound. See comment in v1.

I extended the psock_fanout test with unbound fanout test.

As far as I understand, the easiest way to verify bind is to test that
po->prot_hook.dev != NULL, since we are under a bind_lock anyway.
But perhaps a more readable and direct approach to test "bind" would be
to test po->ifindex != -1, as ifindex is commented as "bound device".
However, at the moment ifindex is not initialized to -1, I can add such
initialization, but perhaps I do not fully understand all the logic.

Any preferences?

>
>>  		err = -ENOSPC;
>>  		if (refcount_read(&match->sk_ref) < match->max_num_members) {
>> -			__dev_remove_pack(&po->prot_hook);
>> -
>>  			/* Paired with packet_setsockopt(PACKET_FANOUT_DATA) */
>>  			WRITE_ONCE(po->fanout, match);
>>
>>  			po->rollover = rollover;
>>  			rollover = NULL;
>>  			refcount_set(&match->sk_ref, refcount_read(&match->sk_ref) + 1);
>> -			__fanout_link(sk, po);
>> +			if (packet_sock_flag(po, PACKET_SOCK_RUNNING)) {
>> +				__dev_remove_pack(&po->prot_hook);
>> +				__fanout_link(sk, po);
>> +			}
>>  			err = 0;
>>  		}
>>  	}
> --