Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs

[James Prestwood <prestwoj@gmail.com>]

On 5/8/23 11:55 AM, James Prestwood wrote:
> Hi Denis,
> 
> On 5/8/23 11:05 AM, Denis Kenzior wrote:
>> Hi James,
>>
>>>> Second, if we're passing unicast frames, and the unicast frame is 
>>>> addressed to a radio in a different namespace, how does that radio 
>>>> get the frame?  We don't have it in our radio_info queue?
>>>
>>> The namespace bars hwsim from seeing radios in other namespaces via 
>>> GET_WIPHY. So yes, the radios are not in the radio_info queue. But 
>>> since the radios are on the same kernel any MAC the kernel knows 
>>> about is accepted when sending a frame, regardless of namespace.
>>>
>>
>> I get this part.  What bothers me is that the current dispatch code 
>> seems to be completely wrong?
>>
>>>>
>>>> The comment for the code you're taking out makes it clear that the 
>>>> intent is for unicast frames to be sent only only to the radio with 
>>>> the target receiver address.  And the optimization is simply about 
>>>> not sending frames to radios with addresses we know do not have the 
>>>> target address.
>>>>
>>>> What you're doing here seems to indicate that our implementation 
>>>> (and the comment) is completely bogus.  And we can simply send any 
>>>> frame to any radio instance (once) and the frames will be forwarded 
>>>> automagically.  If so, then you might want to explain this better 
>>>> and fix the code to reflect this.
>>>
>>> Maybe I described this badly, but I was trying to make 2 points:
>>>
>>>   - Historically, before namespaces, hwsim should never be in a 
>>> situation where a frame is addressed to an unknown recipient. Hostapd 
>>> won't do this and IWD won't do this. So all that code was doing is 
>>> looping through the radios. Maybe there is some case I haven't 
>>> thought of, in which case the kernel would drop the frame anyways.
>>
>> Sure.  To put it another way, hwsim works today by doing roughly the 
>> following:
>>
>> Suppose we have a setup with a single namespace, with 3 radios.
>>
>> Radio A with address aa
>> Radio B with address bb
>> Radio C with address cc
>>
>> When a _unicast_ frame with target 'cc' is seen by hwsim, it loops 
>> over A, B, C and compares the addresses.  If the addresses do not 
>> match, then no HWSIM_CMD_FRAME is issued for that radio.  So we'd only 
>> issue HWSIM_CMD_FRAME with Radio C's HWSIM_ATTR_ADDR_RECEIVER.
> 
> Ah ok, I see where you're coming from now. I overlooked that part. I can 
> re-work this and if the destination is a known radio then only forward 
> to that radio. If the destination is to an unknown radio I think we have 
> to forward it regardless.
> 
>>
>> What you're doing in this patch is to now issue the frame to each and 
>> every known radio.
>>
>> So suppose you change this to 2 namespaces, with Radio A+B in 
>> namespace A, Radio C in namespace C, etc.  Are you running 2 instances 
>> of hwsim? One per namespace?
> 
> Yes, a separate hwsim instance per-namespace.
> 
>>
>> How does it help if namespace A receives a frame with target 'CC'? It 
>> doesn't know about Radio C, so how would radio C even receive the 
>> message? And if it somehow does anyway, then it would seem like the 
>> above dispatch logic is bogus in the first place?
> 
> If a client in namespace A sends a frame to target CC then the hwsim 
> instance in namespace A gets this frame. It doesn't know about target CC 
> but the kernel does. So it forwards into the kernel and target CC 
> receives it.
> 
> I don't think there is anything wrong with the dispatch logic. There is 
> just a disconnect between what information hwsim knows vs the kernel. I 
> think tweaking the optimization I removed (described above) effectively 
> keeps things the same (assuming clients are sane and don't send frames 
> to completely bogus addresses).

Ok, actually I see your confusion now. Removing that block of code would 
send the frame to all *known* radios only... So this actually should not 
work, but it does.

Its like the kernel is disregarding ATTR_RECEIVER_ADDR and just looking 
at the frame contents. Anyways, I'll investigate.

> 
> 
>>
>>>
>>>   - Then with the introduction of namespaces we actually do hit this 
>>> case of having unknown destination addresses. Not because they are 
>>> bogus, but because hwsim simply cannot know about radios in other 
>>> namespaces.
>>>
>>
>> Right, I get that.  But this is not what my question is about :)
>>
>> Regards,
>> -Denis