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

[James Prestwood <prestwoj@gmail.com>]

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).


> 
>>
>>   - 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