* [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
@ 2023-05-04 21:52 James Prestwood
2023-05-04 21:52 ` [PATCH 2/3] test-runner: allow hwsim in namespaces James Prestwood
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: James Prestwood @ 2023-05-04 21:52 UTC (permalink / raw)
To: iwd; +Cc: James Prestwood
Based on the comment hwsim would only send frames to known MACs
which on its face seems reasonable. But in reality there shouldn't
ever be frames going to unknown MACs, at least not unknown to the
kernel. We can hit this case, though, when using network namespaces.
Each namespace is siloed and hwsim instances only know about radios
in that namespace.
In order to support hwsim and namespaces, this restriction is
being removed.
---
tools/hwsim.c | 33 ---------------------------------
1 file changed, 33 deletions(-)
diff --git a/tools/hwsim.c b/tools/hwsim.c
index 47352ad4..10a9db5f 100644
--- a/tools/hwsim.c
+++ b/tools/hwsim.c
@@ -1501,43 +1501,10 @@ static void process_frame(struct hwsim_frame *frame)
struct send_frame_info *send_info;
bool drop = drop_mcast;
uint32_t delay = 0;
- const struct l_queue_entry *i;
if (radio == frame->src_radio)
continue;
- /*
- * The kernel hwsim medium passes multicast frames to all
- * radios that are on the same frequency as this frame but
- * the netlink medium API only lets userspace pass frames to
- * radios by known hardware address. It does check that the
- * receiving radio is on the same frequency though so we can
- * send to all known addresses.
- *
- * If the frame's Receiver Address (RA) is a multicast
- * address, then send the frame to every radio that is
- * registered. If it's a unicast address then optimize
- * by only forwarding the frame to the radios that have
- * at least one interface with this specific address.
- */
- if (!util_is_broadcast_address(frame->dst_ether_addr)) {
- for (i = l_queue_get_entries(interface_info);
- i; i = i->next) {
- struct interface_info_rec *interface = i->data;
-
- if (interface->radio_rec != radio)
- continue;
-
- if (!memcmp(interface->addr,
- frame->dst_ether_addr,
- ETH_ALEN))
- break;
- }
-
- if (!i)
- continue;
- }
-
process_rules(frame->src_radio, radio, frame, false,
&drop, &delay);
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] test-runner: allow hwsim in namespaces
2023-05-04 21:52 [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs James Prestwood
@ 2023-05-04 21:52 ` James Prestwood
2023-05-04 21:52 ` [PATCH 3/3] test-runner: fix __str__ for namespace processes James Prestwood
2023-05-07 23:21 ` [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs Denis Kenzior
2 siblings, 0 replies; 27+ messages in thread
From: James Prestwood @ 2023-05-04 21:52 UTC (permalink / raw)
To: iwd; +Cc: James Prestwood
Starts the hwsim daemon per-namespace rather than only in the
root namespace.
---
tools/run-tests | 16 ++++++++--------
tools/utils.py | 13 ++++++++++++-
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/tools/run-tests b/tools/run-tests
index 32c09723..ef316666 100755
--- a/tools/run-tests
+++ b/tools/run-tests
@@ -319,6 +319,7 @@ class TestContext(Namespace):
self.namespaces = []
self._last_mem_available = 0
self._mem_chart = BarChart()
+ self.register_hwsim = False;
def start_dbus_monitor(self):
if not Process.is_verbose('dbus-monitor'):
@@ -329,18 +330,16 @@ class TestContext(Namespace):
def start_haveged(self):
self.start_process(['haveged', '-F'])
+ def start_hwsim(self):
+ self.register_hwsim = self.hw_config['SETUP'].get('hwsim_medium', 'no') in ['no', '0', 'false']
+
+ super().start_hwsim(self.register_hwsim)
+
def create_radios(self):
setup = self.hw_config['SETUP']
nradios = int(setup['num_radios'])
args = ['hwsim']
- if self.hw_config['SETUP'].get('hwsim_medium', 'no') in ['no', '0', 'false']:
- # register hwsim as medium
- args.extend(['--no-register'])
-
- proc = self.start_process(args)
- proc.wait_for_service(self, 'net.connman.hwsim', 20)
-
for i in range(nradios):
name = 'rad%u' % i
@@ -524,7 +523,7 @@ class TestContext(Namespace):
# Remove radios from 'root' namespace
self.radios = list(set(self.radios) - set(radios))
- self.namespaces.append(Namespace(self.args, key, radios))
+ self.namespaces.append(Namespace(self.args, key, radios, self.register_hwsim))
def get_namespace(self, ns):
for n in self.namespaces:
@@ -865,6 +864,7 @@ def pre_test(ctx, test, copied):
ctx.start_dbus()
ctx.start_haveged()
ctx.start_dbus_monitor()
+ ctx.start_hwsim()
ctx.start_radios()
ctx.create_namespaces()
ctx.start_hostapd()
diff --git a/tools/utils.py b/tools/utils.py
index a07c3183..d2e1c33b 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -320,7 +320,7 @@ busconfig.dtd\">
'''
class Namespace:
- def __init__(self, args, name, radios):
+ def __init__(self, args, name, radios, hwsim_register=False):
self.dbus_address = None
self.name = name
self.radios = radios
@@ -331,6 +331,7 @@ class Namespace:
r.set_namespace(self)
self.start_dbus()
+ self.start_hwsim(register=hwsim_register)
def reset(self):
self._bus = None
@@ -458,6 +459,16 @@ class Namespace:
return proc
+ def start_hwsim(self, register=False):
+ args = ['hwsim']
+
+ if register:
+ # register hwsim as medium
+ args.extend(['--no-register'])
+
+ proc = self.start_process(args)
+ proc.wait_for_service(self, 'net.connman.hwsim', 20)
+
@staticmethod
def non_block_wait(func, timeout, *args, exception=True):
'''
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/3] test-runner: fix __str__ for namespace processes
2023-05-04 21:52 [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs James Prestwood
2023-05-04 21:52 ` [PATCH 2/3] test-runner: allow hwsim in namespaces James Prestwood
@ 2023-05-04 21:52 ` James Prestwood
2023-05-07 23:21 ` [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs Denis Kenzior
2 siblings, 0 replies; 27+ messages in thread
From: James Prestwood @ 2023-05-04 21:52 UTC (permalink / raw)
To: iwd; +Cc: James Prestwood
When printing out a namespace the Process.get_all() call was
returning all processes running, even those not in the namespace
being printed. Modifying get_all() is tricky since its a class
method (and used elsewhere for ALL processes) so instead filter
the results to only show processes in the namespace being printed.
---
tools/utils.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/utils.py b/tools/utils.py
index d2e1c33b..48d5d219 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -529,7 +529,8 @@ class Namespace:
ret = 'Namespace: %s\n' % self.name
ret += 'Processes:\n'
for p in Process.get_all():
- ret += '\t%s' % str(p)
+ if p.namespace == self.name:
+ ret += '\t%s' % str(p)
ret += 'Radios:\n'
if len(self.radios) > 0:
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-04 21:52 [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs James Prestwood
2023-05-04 21:52 ` [PATCH 2/3] test-runner: allow hwsim in namespaces James Prestwood
2023-05-04 21:52 ` [PATCH 3/3] test-runner: fix __str__ for namespace processes James Prestwood
@ 2023-05-07 23:21 ` Denis Kenzior
2023-05-08 13:43 ` James Prestwood
2 siblings, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-05-07 23:21 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
On 5/4/23 16:52, James Prestwood wrote:
> Based on the comment hwsim would only send frames to known MACs
> which on its face seems reasonable. But in reality there shouldn't
> ever be frames going to unknown MACs, at least not unknown to the
> kernel. We can hit this case, though, when using network namespaces.
> Each namespace is siloed and hwsim instances only know about radios
> in that namespace.
First, can you explain what is the motivation behind this series?
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 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.
>
> In order to support hwsim and namespaces, this restriction is
> being removed.
> ---
> tools/hwsim.c | 33 ---------------------------------
> 1 file changed, 33 deletions(-)
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-07 23:21 ` [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs Denis Kenzior
@ 2023-05-08 13:43 ` James Prestwood
2023-05-08 18:05 ` Denis Kenzior
0 siblings, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-05-08 13:43 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 5/7/23 4:21 PM, Denis Kenzior wrote:
> Hi James,
>
> On 5/4/23 16:52, James Prestwood wrote:
>> Based on the comment hwsim would only send frames to known MACs
>> which on its face seems reasonable. But in reality there shouldn't
>> ever be frames going to unknown MACs, at least not unknown to the
>> kernel. We can hit this case, though, when using network namespaces.
>> Each namespace is siloed and hwsim instances only know about radios
>> in that namespace.
>
> First, can you explain what is the motivation behind this series?
I'd like to enable hwsim to work with namespaces. First in order to test
the roam before netconfig changes. And maybe future tests would need this.
>
> 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.
>
> 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.
- 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.
Thanks,
James
>
>>
>> In order to support hwsim and namespaces, this restriction is
>> being removed.
>> ---
>> tools/hwsim.c | 33 ---------------------------------
>> 1 file changed, 33 deletions(-)
>>
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-08 13:43 ` James Prestwood
@ 2023-05-08 18:05 ` Denis Kenzior
2023-05-08 18:55 ` James Prestwood
0 siblings, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-05-08 18:05 UTC (permalink / raw)
To: James Prestwood, iwd
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.
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?
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?
>
> - 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-08 18:05 ` Denis Kenzior
@ 2023-05-08 18:55 ` James Prestwood
2023-05-08 19:00 ` Denis Kenzior
2023-05-08 19:03 ` James Prestwood
0 siblings, 2 replies; 27+ messages in thread
From: James Prestwood @ 2023-05-08 18:55 UTC (permalink / raw)
To: Denis Kenzior, iwd
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-08 18:55 ` James Prestwood
@ 2023-05-08 19:00 ` Denis Kenzior
2023-05-08 19:03 ` James Prestwood
1 sibling, 0 replies; 27+ messages in thread
From: Denis Kenzior @ 2023-05-08 19:00 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>>
>> 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.
>
Well, that's the part that is unclear to me. See below.
>>
>> 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.
So suppose client in namespace A sends something via Radio A to Radio C. hwsim
instance in namespace A will loop over the radios it knows about, which are A
and B. A is the source radio, so that should be ignored. So with your proposed
logic, hwsim in namespace A will send the frame to radio B with the target being
Radio C. But that would seem to be pointless? If sending to radio B regardless
of the target address is fine, why do we even bother with the dispatch logic at all?
Are you sure it isn't being sent to hwsim instances in all namespaces?
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-08 19:03 ` James Prestwood
@ 2023-05-08 19:01 ` Denis Kenzior
2023-06-21 21:05 ` James Prestwood
0 siblings, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-05-08 19:01 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>
> 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.
Yep! Exactly.
>
> Its like the kernel is disregarding ATTR_RECEIVER_ADDR and just looking at the
> frame contents. Anyways, I'll investigate.
>
Ok, great.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-08 18:55 ` James Prestwood
2023-05-08 19:00 ` Denis Kenzior
@ 2023-05-08 19:03 ` James Prestwood
2023-05-08 19:01 ` Denis Kenzior
1 sibling, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-05-08 19:03 UTC (permalink / raw)
To: Denis Kenzior, iwd
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-05-08 19:01 ` Denis Kenzior
@ 2023-06-21 21:05 ` James Prestwood
2023-06-27 2:31 ` Denis Kenzior
0 siblings, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-06-21 21:05 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 5/8/23 12:01 PM, Denis Kenzior wrote:
> Hi James,
>
>>
>> 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.
>
> Yep! Exactly.
>
>>
>> Its like the kernel is disregarding ATTR_RECEIVER_ADDR and just
>> looking at the frame contents. Anyways, I'll investigate.
>>
>
> Ok, great.
Just an update here. The reason hwsim was behaving like this was because
DEL_WIPHY isn't handled but DEL_INTERFACE is.
So when hwsim starts it dumps and tracks all the radios but when a radio
gets moved into another namespace only the interfaces get cleaned up.
Then a frame comes and hwsim won't find a matching interface->addr to
send the frame, and bail out. But when I removed that block of code it
wouldn't bail out and since the radios still existed in the queue the
frame could be sent out to all radios including "removed" radios. Since
the kernel doesn't care about namespaces those "removed" radios still
got the frames.
So at least that's sorted out. How we want to support namespaces is the
question. A few options:
- Peek into the frame at the destination address and use that as
ATTR_RECEIVER, but this won't work with address randomization for phys
outside of the root namespace. IMO this is probably fine for testing
purposes, if we know about it ahead of time.
- Add support to hwsim to move radios to other namespaces, and keep
track of those radios (similar to what we erroneously have today). This
is fragile though because you no longer can remove radios/interfaces,
nor do you know if they have been removed.
Thanks,
James
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-21 21:05 ` James Prestwood
@ 2023-06-27 2:31 ` Denis Kenzior
2023-06-27 15:15 ` James Prestwood
0 siblings, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-06-27 2:31 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
> Just an update here. The reason hwsim was behaving like this was because
> DEL_WIPHY isn't handled but DEL_INTERFACE is.
>
Funny.
> So when hwsim starts it dumps and tracks all the radios but when a radio gets
> moved into another namespace only the interfaces get cleaned up. Then a frame
> comes and hwsim won't find a matching interface->addr to send the frame, and
> bail out. But when I removed that block of code it wouldn't bail out and since
> the radios still existed in the queue the frame could be sent out to all radios
> including "removed" radios. Since the kernel doesn't care about namespaces those
> "removed" radios still got the frames.
Ok, that makes sense now.
>
> So at least that's sorted out. How we want to support namespaces is the
> question. A few options:
>
> - Peek into the frame at the destination address and use that as ATTR_RECEIVER,
> but this won't work with address randomization for phys outside of the root
> namespace. IMO this is probably fine for testing purposes, if we know about it
> ahead of time.
You can try to see whether HWSIM_CMD_ADD_MAC_ADDR works? See commit
5cc58a9ecfa1 ("mac80211_hwsim: notify wmediumd of used MAC addresses")
>
> - Add support to hwsim to move radios to other namespaces, and keep track of
> those radios (similar to what we erroneously have today). This is fragile though
> because you no longer can remove radios/interfaces, nor do you know if they have
> been removed.
Hmm, but radios are namespace independent. They can only be added/removed via
HWSIM_CMD_ADD/DEL_RADIO, no? Since phys are moved wholesale across namespaces
(you can't only move a given interface), you could assume that once a radio is
created and populated, its interfaces do not change for the duration of the
test, even if they're moved to a different namespace.
Or if the HWSIM_CMD_ADD_MAC_ADDR API works, that might make things even simpler.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-27 2:31 ` Denis Kenzior
@ 2023-06-27 15:15 ` James Prestwood
2023-06-27 18:00 ` Denis Kenzior
0 siblings, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-06-27 15:15 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 6/26/23 7:31 PM, Denis Kenzior wrote:
> Hi James,
>
>> Just an update here. The reason hwsim was behaving like this was
>> because DEL_WIPHY isn't handled but DEL_INTERFACE is.
>>
>
> Funny.
>
>> So when hwsim starts it dumps and tracks all the radios but when a
>> radio gets moved into another namespace only the interfaces get
>> cleaned up. Then a frame comes and hwsim won't find a matching
>> interface->addr to send the frame, and bail out. But when I removed
>> that block of code it wouldn't bail out and since the radios still
>> existed in the queue the frame could be sent out to all radios
>> including "removed" radios. Since the kernel doesn't care about
>> namespaces those "removed" radios still got the frames.
>
> Ok, that makes sense now.
>
>>
>> So at least that's sorted out. How we want to support namespaces is
>> the question. A few options:
>>
>> - Peek into the frame at the destination address and use that as
>> ATTR_RECEIVER, but this won't work with address randomization for phys
>> outside of the root namespace. IMO this is probably fine for testing
>> purposes, if we know about it ahead of time.
>
> You can try to see whether HWSIM_CMD_ADD_MAC_ADDR works? See commit
> 5cc58a9ecfa1 ("mac80211_hwsim: notify wmediumd of used MAC addresses")
I hadn't seen that before, but looking at it they don't expose that as
an API, its only for internal use for scan address randomization and
monitor interfaces (see mac80211_hwsim_config_mac_nl()).
>
>>
>> - Add support to hwsim to move radios to other namespaces, and keep
>> track of those radios (similar to what we erroneously have today).
>> This is fragile though because you no longer can remove
>> radios/interfaces, nor do you know if they have been removed.
>
> Hmm, but radios are namespace independent. They can only be
> added/removed via HWSIM_CMD_ADD/DEL_RADIO, no? Since phys are moved
> wholesale across namespaces (you can't only move a given interface), you
> could assume that once a radio is created and populated, its interfaces
> do not change for the duration of the test, even if they're moved to a
> different namespace.
For testing yes this is probably fine. It may require some adaptation in
hwsim to do it better from a test-runner perspective. Currently we just
use ip to create/delete namespaces and move radios. It may make more
sense to add this to hwsim so there is one path. Then at least when
hwsim gets a DEL_WIPHY event it doesn't have to assume the radio was
moved (I'm thinking if we ever added hotplug tests this could be important).
I'll pursue this path.
>
> Or if the HWSIM_CMD_ADD_MAC_ADDR API works, that might make things even
> simpler.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-27 15:15 ` James Prestwood
@ 2023-06-27 18:00 ` Denis Kenzior
2023-06-27 18:56 ` James Prestwood
2023-06-27 20:09 ` James Prestwood
0 siblings, 2 replies; 27+ messages in thread
From: Denis Kenzior @ 2023-06-27 18:00 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>> You can try to see whether HWSIM_CMD_ADD_MAC_ADDR works? See commit
>> 5cc58a9ecfa1 ("mac80211_hwsim: notify wmediumd of used MAC addresses")
>
> I hadn't seen that before, but looking at it they don't expose that as an API,
> its only for internal use for scan address randomization and monitor interfaces
> (see mac80211_hwsim_config_mac_nl()).
>
It is used as an unsolicited event / notification. Yeah I know, HWSIM_CMD_*
prefix is confusing ;)
It looks like we should start using this since most of our autotests are forced
to include the following main.conf:
[Scan]
DisableMacAddressRandomization=true
Having support for the above event would allow us to get rid of this hack.
<snip>
>>
>> Hmm, but radios are namespace independent. They can only be added/removed via
>> HWSIM_CMD_ADD/DEL_RADIO, no? Since phys are moved wholesale across namespaces
>> (you can't only move a given interface), you could assume that once a radio is
>> created and populated, its interfaces do not change for the duration of the
>> test, even if they're moved to a different namespace.
>
> For testing yes this is probably fine. It may require some adaptation in hwsim
> to do it better from a test-runner perspective. Currently we just use ip to
> create/delete namespaces and move radios. It may make more sense to add this to
> hwsim so there is one path. Then at least when hwsim gets a DEL_WIPHY event it
It shouldn't matter really who invokes the namespace move. hwsim would know
whether it is a hot-unplug or a namespace move by virtue of being the one who
triggers HWSIM_CMD_DEL_RADIO.
> doesn't have to assume the radio was moved (I'm thinking if we ever added
> hotplug tests this could be important).
We should add these, but the above still stands. hwsim is the only thing in our
tests that triggers HWSIM_CMD_ADD_RADIO.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-27 18:00 ` Denis Kenzior
@ 2023-06-27 18:56 ` James Prestwood
2023-06-27 19:23 ` Denis Kenzior
2023-06-27 20:09 ` James Prestwood
1 sibling, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-06-27 18:56 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 6/27/23 11:00 AM, Denis Kenzior wrote:
> Hi James,
>
>>> You can try to see whether HWSIM_CMD_ADD_MAC_ADDR works? See commit
>>> 5cc58a9ecfa1 ("mac80211_hwsim: notify wmediumd of used MAC addresses")
>>
>> I hadn't seen that before, but looking at it they don't expose that as
>> an API, its only for internal use for scan address randomization and
>> monitor interfaces (see mac80211_hwsim_config_mac_nl()).
>>
>
> It is used as an unsolicited event / notification. Yeah I know,
> HWSIM_CMD_* prefix is confusing ;)
>
> It looks like we should start using this since most of our autotests are
> forced to include the following main.conf:
>
> [Scan]
> DisableMacAddressRandomization=true
>
> Having support for the above event would allow us to get rid of this hack.
IIRC Tim started adding that into tests a long time ago, and had said
improves reliability. If I remove it (e.g. in testPSK-roam) things start
breaking, but its very random and not repeatable, some pass and some
fail between runs.
I would expect all hwsim tests to fail if we are dropping all frames
with and unknown TX or RX address so something else must be going on
here. But indeed, this *should* work if we handle these events similarly
to wmediumd.
>
> <snip>
>
>>>
>>> Hmm, but radios are namespace independent. They can only be
>>> added/removed via HWSIM_CMD_ADD/DEL_RADIO, no? Since phys are moved
>>> wholesale across namespaces (you can't only move a given interface),
>>> you could assume that once a radio is created and populated, its
>>> interfaces do not change for the duration of the test, even if
>>> they're moved to a different namespace.
>>
>> For testing yes this is probably fine. It may require some adaptation
>> in hwsim to do it better from a test-runner perspective. Currently we
>> just use ip to create/delete namespaces and move radios. It may make
>> more sense to add this to hwsim so there is one path. Then at least
>> when hwsim gets a DEL_WIPHY event it
>
> It shouldn't matter really who invokes the namespace move. hwsim would
> know whether it is a hot-unplug or a namespace move by virtue of being
> the one who triggers HWSIM_CMD_DEL_RADIO.
>
>> doesn't have to assume the radio was moved (I'm thinking if we ever
>> added hotplug tests this could be important).
>
> We should add these, but the above still stands. hwsim is the only
> thing in our tests that triggers HWSIM_CMD_ADD_RADIO.
Yeah true, I'll just flag the radio as moved but keep it around for
processing frames.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-27 18:56 ` James Prestwood
@ 2023-06-27 19:23 ` Denis Kenzior
0 siblings, 0 replies; 27+ messages in thread
From: Denis Kenzior @ 2023-06-27 19:23 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>> It looks like we should start using this since most of our autotests are
>> forced to include the following main.conf:
>>
>> [Scan]
>> DisableMacAddressRandomization=true
>>
>> Having support for the above event would allow us to get rid of this hack.
> IIRC Tim started adding that into tests a long time ago, and had said improves
> reliability. If I remove it (e.g. in testPSK-roam) things start breaking, but
> its very random and not repeatable, some pass and some fail between runs.
Yep. It is very random because the active scans via probe requests fail, but
the beacons still come in. So it is very much timing/luck dependent whether a
scan picks up the network the test is interested in.
We started noticing the failures shortly after adding scan address randomization
support. 'git blame' pointed the finger, so we added this setting to work
around the issue. Seems blindly obvious now, but we didn't recognize the link
between address randomization and hwsim at that time.
>
> I would expect all hwsim tests to fail if we are dropping all frames with and
> unknown TX or RX address so something else must be going on here. But indeed,
> this *should* work if we handle these events similarly to wmediumd.
See above about beacons.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-27 18:00 ` Denis Kenzior
2023-06-27 18:56 ` James Prestwood
@ 2023-06-27 20:09 ` James Prestwood
2023-06-28 14:49 ` Denis Kenzior
1 sibling, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-06-27 20:09 UTC (permalink / raw)
To: Denis Kenzior, iwd
On 6/27/23 11:00 AM, Denis Kenzior wrote:
> Hi James,
>
>>> You can try to see whether HWSIM_CMD_ADD_MAC_ADDR works? See commit
>>> 5cc58a9ecfa1 ("mac80211_hwsim: notify wmediumd of used MAC addresses")
>>
>> I hadn't seen that before, but looking at it they don't expose that as
>> an API, its only for internal use for scan address randomization and
>> monitor interfaces (see mac80211_hwsim_config_mac_nl()).
>>
>
> It is used as an unsolicited event / notification. Yeah I know,
> HWSIM_CMD_* prefix is confusing ;)
>
> It looks like we should start using this since most of our autotests are
> forced to include the following main.conf:
>
> [Scan]
> DisableMacAddressRandomization=true
>
> Having support for the above event would allow us to get rid of this hack.
>
> <snip>
>
>>>
>>> Hmm, but radios are namespace independent. They can only be
>>> added/removed via HWSIM_CMD_ADD/DEL_RADIO, no? Since phys are moved
>>> wholesale across namespaces (you can't only move a given interface),
>>> you could assume that once a radio is created and populated, its
>>> interfaces do not change for the duration of the test, even if
>>> they're moved to a different namespace.
>>
>> For testing yes this is probably fine. It may require some adaptation
>> in hwsim to do it better from a test-runner perspective. Currently we
>> just use ip to create/delete namespaces and move radios. It may make
>> more sense to add this to hwsim so there is one path. Then at least
>> when hwsim gets a DEL_WIPHY event it
>
> It shouldn't matter really who invokes the namespace move. hwsim would
> know whether it is a hot-unplug or a namespace move by virtue of being
> the one who triggers HWSIM_CMD_DEL_RADIO.
So one other caveat with this is that IWD creates its interface while
inside the namespace. So we actually cannot track that. Only thing we
could do is force the interface creation ahead of time, and make
namespace'd iwd's use the default interface.
>
>> doesn't have to assume the radio was moved (I'm thinking if we ever
>> added hotplug tests this could be important).
>
> We should add these, but the above still stands. hwsim is the only
> thing in our tests that triggers HWSIM_CMD_ADD_RADIO.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-27 20:09 ` James Prestwood
@ 2023-06-28 14:49 ` Denis Kenzior
2023-06-28 15:33 ` James Prestwood
0 siblings, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-06-28 14:49 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>
> So one other caveat with this is that IWD creates its interface while inside the
> namespace. So we actually cannot track that. Only thing we could do is force the
> interface creation ahead of time, and make namespace'd iwd's use the default
> interface.
>
Yep. Since iwd will have to handle multi-mode devices and multiple interfaces,
there's really not much we can do in the general case. This is why we need to
pay attention to HWSIM_CMD_ADD_MAC_ADDR.
Looking at that kernel commit again, it seems to already invoke
mac80211_hwsim_config_mac_nl() on add_interface and remove_interface. So maybe
everything needed to handle interface creation/removal inside namespaces is
already there? If not, then any missing event generation should be fixed in the
kernel.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 14:49 ` Denis Kenzior
@ 2023-06-28 15:33 ` James Prestwood
2023-06-28 15:40 ` Denis Kenzior
0 siblings, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-06-28 15:33 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 6/28/23 7:49 AM, Denis Kenzior wrote:
> Hi James,
>
>>
>> So one other caveat with this is that IWD creates its interface while
>> inside the namespace. So we actually cannot track that. Only thing we
>> could do is force the interface creation ahead of time, and make
>> namespace'd iwd's use the default interface.
>>
>
> Yep. Since iwd will have to handle multi-mode devices and multiple
> interfaces, there's really not much we can do in the general case. This
> is why we need to pay attention to HWSIM_CMD_ADD_MAC_ADDR.
>
> Looking at that kernel commit again, it seems to already invoke
> mac80211_hwsim_config_mac_nl() on add_interface and remove_interface.
> So maybe everything needed to handle interface creation/removal inside
> namespaces is already there? If not, then any missing event generation
> should be fixed in the kernel.
Unfortunately once you move the radio into another namespace you lose
any events associated with that radio, due to the PID of hwsim being
associated with a distinct namespace (I think). So ADD/DEL_MAC_ADDR
won't help here.
But still, its nice getting the ADD/DEL_MAC_ADDR events purely for
supporting address randomization.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 15:33 ` James Prestwood
@ 2023-06-28 15:40 ` Denis Kenzior
2023-06-28 16:14 ` James Prestwood
0 siblings, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-06-28 15:40 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>
> Unfortunately once you move the radio into another namespace you lose any events
> associated with that radio, due to the PID of hwsim being associated with a
> distinct namespace (I think). So ADD/DEL_MAC_ADDR won't help here.
Which events though? You should always be getting HWSIM_CMD_ events for _all_
radios since only a single hwsim instance is registered, no? Aren't these all
you need for the purposes of sending packets to the right radio?
The NL80211 WIPHY events aren't really needed. We only use these for
informational purposes (only the name is used, I think?) I guess we also use
this information for the radio_ap_only() optimization we do, but we could
probably solve this another way.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 15:40 ` Denis Kenzior
@ 2023-06-28 16:14 ` James Prestwood
2023-06-28 16:25 ` Denis Kenzior
2023-06-28 23:19 ` Andrew Zaborowski
0 siblings, 2 replies; 27+ messages in thread
From: James Prestwood @ 2023-06-28 16:14 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 6/28/23 8:40 AM, Denis Kenzior wrote:
> Hi James,
>
>>
>> Unfortunately once you move the radio into another namespace you lose
>> any events associated with that radio, due to the PID of hwsim being
>> associated with a distinct namespace (I think). So ADD/DEL_MAC_ADDR
>> won't help here.
>
> Which events though? You should always be getting HWSIM_CMD_ events for
> _all_ radios since only a single hwsim instance is registered, no?
> Aren't these all you need for the purposes of sending packets to the
> right radio?
ADD/DEL_MAC_ADDR, which don't come once the radio is moved to another
namespace (as far as I can tell with testing).
I think it comes down to port ID:
u32 _portid = READ_ONCE(data->wmediumd);
...
hwsim_unicast_netgroup(data, skb, _portid);
One other potential (but much more complex) solution would be to use
multiple hwsim instances that communicate, making a sort of distributed
hwsim environment. I'm working on a similar concept which could
(partially) be upstreamed, at least the hwsim parts.
We could provide hwsim a socket/FD to communicate with other hwsim
instances. hwsim would behave the same as it does now unless it receives
a frame which doesn't match to any known radios. In which case it send
that frame out to the external socket. It would also receive on that
socket, and pass the frame back to its local hwsim socket.
I have it now where I just use the netlink protocol between hwsim
instances, so you can receive on the remote socket/FD and send that
directly back to the kernel for the local instance.
This is similar enough that some of what I've done already could be
upstreamed and solve this namespace problem.
Thanks,
James
>
> The NL80211 WIPHY events aren't really needed. We only use these for
> informational purposes (only the name is used, I think?) I guess we
> also use this information for the radio_ap_only() optimization we do,
> but we could probably solve this another way.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 16:14 ` James Prestwood
@ 2023-06-28 16:25 ` Denis Kenzior
2023-06-28 16:47 ` James Prestwood
2023-06-28 23:19 ` Andrew Zaborowski
1 sibling, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-06-28 16:25 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>>> Unfortunately once you move the radio into another namespace you lose any
>>> events associated with that radio, due to the PID of hwsim being associated
>>> with a distinct namespace (I think). So ADD/DEL_MAC_ADDR won't help here.
>>
>> Which events though? You should always be getting HWSIM_CMD_ events for _all_
>> radios since only a single hwsim instance is registered, no? Aren't these all
>> you need for the purposes of sending packets to the right radio?
>
> ADD/DEL_MAC_ADDR, which don't come once the radio is moved to another namespace
> (as far as I can tell with testing).
>
> I think it comes down to port ID:
>
> u32 _portid = READ_ONCE(data->wmediumd);
> ...
> hwsim_unicast_netgroup(data, skb, _portid);
Then this may be a conversation you have to start on linux-wireless. I would
have thought that there should only be a single wmediumd instance on a system
regardless of namespaces, but maybe I'm wrong here.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 16:25 ` Denis Kenzior
@ 2023-06-28 16:47 ` James Prestwood
2023-06-28 16:57 ` Denis Kenzior
0 siblings, 1 reply; 27+ messages in thread
From: James Prestwood @ 2023-06-28 16:47 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 6/28/23 9:25 AM, Denis Kenzior wrote:
> Hi James,
>
>>>> Unfortunately once you move the radio into another namespace you
>>>> lose any events associated with that radio, due to the PID of hwsim
>>>> being associated with a distinct namespace (I think). So
>>>> ADD/DEL_MAC_ADDR won't help here.
>>>
>>> Which events though? You should always be getting HWSIM_CMD_ events
>>> for _all_ radios since only a single hwsim instance is registered,
>>> no? Aren't these all you need for the purposes of sending packets to
>>> the right radio?
>>
>> ADD/DEL_MAC_ADDR, which don't come once the radio is moved to another
>> namespace (as far as I can tell with testing).
>>
>> I think it comes down to port ID:
>>
>> u32 _portid = READ_ONCE(data->wmediumd);
>> ...
>> hwsim_unicast_netgroup(data, skb, _portid);
>
> Then this may be a conversation you have to start on linux-wireless. I
> would have thought that there should only be a single wmediumd instance
> on a system regardless of namespaces, but maybe I'm wrong here.
Yeah, I'll do that as well. This would simplify things if the namespaces
didn't come into play.
Nevertheless, was the distributed hwsim concept something you'd be
interested in accepting upstream? Like I said, I'm already doing it and
I'm happy to re-use hwsim and extend to support this external socket
concept.
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 16:47 ` James Prestwood
@ 2023-06-28 16:57 ` Denis Kenzior
2023-06-28 17:22 ` James Prestwood
0 siblings, 1 reply; 27+ messages in thread
From: Denis Kenzior @ 2023-06-28 16:57 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>> Then this may be a conversation you have to start on linux-wireless. I would
>> have thought that there should only be a single wmediumd instance on a system
>> regardless of namespaces, but maybe I'm wrong here.
>
> Yeah, I'll do that as well. This would simplify things if the namespaces didn't
> come into play.
Check
100cb9ff40e0 ("mac80211_hwsim: Allow managing radios from non-initial namespaces")
f21e4d8ed16b ("mac80211_hwsim: Allow wmediumd to attach to radios created in its
netns")
From those two commits I get the impression that the intent is to allow a
single wmediumd across namespaces. If for some reason the ADD_MAC_ADDR messages
are not being sent, then I would look into fixing that first.
>
> Nevertheless, was the distributed hwsim concept something you'd be interested in
> accepting upstream? Like I said, I'm already doing it and I'm happy to re-use
> hwsim and extend to support this external socket concept.
I'm not against it, but as you point out, it sounds super complicated. So I'd
have to look into it more once you are ready to share something.
Regards,
-Denis
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 16:57 ` Denis Kenzior
@ 2023-06-28 17:22 ` James Prestwood
0 siblings, 0 replies; 27+ messages in thread
From: James Prestwood @ 2023-06-28 17:22 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 6/28/23 9:57 AM, Denis Kenzior wrote:
> Hi James,
>
>>> Then this may be a conversation you have to start on linux-wireless.
>>> I would have thought that there should only be a single wmediumd
>>> instance on a system regardless of namespaces, but maybe I'm wrong here.
>>
>> Yeah, I'll do that as well. This would simplify things if the
>> namespaces didn't come into play.
>
> Check
> 100cb9ff40e0 ("mac80211_hwsim: Allow managing radios from non-initial
> namespaces")
> f21e4d8ed16b ("mac80211_hwsim: Allow wmediumd to attach to radios
> created in its netns")
Yeah, the second one does mention frames specifically, which I am
getting from radios outside the namespace. But I don't see a difference
in how ADD/DEL_MAC_ADDR works. But yeah, this is something I'll look into.
>
> From those two commits I get the impression that the intent is to allow
> a single wmediumd across namespaces. If for some reason the
> ADD_MAC_ADDR messages are not being sent, then I would look into fixing
> that first.
>
>>
>> Nevertheless, was the distributed hwsim concept something you'd be
>> interested in accepting upstream? Like I said, I'm already doing it
>> and I'm happy to re-use hwsim and extend to support this external
>> socket concept.
>
> I'm not against it, but as you point out, it sounds super complicated.
> So I'd have to look into it more once you are ready to share something.
Ok fair enough.
>
> Regards,
> -Denis
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 16:14 ` James Prestwood
2023-06-28 16:25 ` Denis Kenzior
@ 2023-06-28 23:19 ` Andrew Zaborowski
2023-06-28 23:28 ` James Prestwood
1 sibling, 1 reply; 27+ messages in thread
From: Andrew Zaborowski @ 2023-06-28 23:19 UTC (permalink / raw)
To: James Prestwood; +Cc: Denis Kenzior, iwd
Hi James,
On Wed, 28 Jun 2023 at 18:14, James Prestwood <prestwoj@gmail.com> wrote:
> One other potential (but much more complex) solution would be to use
> multiple hwsim instances that communicate, making a sort of distributed
> hwsim environment. I'm working on a similar concept which could
> (partially) be upstreamed, at least the hwsim parts.
Another option would be to occasionally dump information about phys
and radios in all namespaces. While this is complicated it may be a
little easier than having multiple hwsim instances because you can
move threads in a single process into network namespaces. I.e. you
can, for example, share an l_queue between temporary threads each of
which moves into one network namespace, opens a netlink socket, dumps
the phy information and exits. Or you can keep the threads alive to
track netlink events but locking becomes more complicated.
Best regards
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs
2023-06-28 23:19 ` Andrew Zaborowski
@ 2023-06-28 23:28 ` James Prestwood
0 siblings, 0 replies; 27+ messages in thread
From: James Prestwood @ 2023-06-28 23:28 UTC (permalink / raw)
To: Andrew Zaborowski; +Cc: Denis Kenzior, iwd
Hi Andrew,
Hi Andrew,
On 6/28/23 4:19 PM, Andrew Zaborowski wrote:
> Hi James,
>
> On Wed, 28 Jun 2023 at 18:14, James Prestwood <prestwoj@gmail.com> wrote:
>> One other potential (but much more complex) solution would be to use
>> multiple hwsim instances that communicate, making a sort of distributed
>> hwsim environment. I'm working on a similar concept which could
>> (partially) be upstreamed, at least the hwsim parts.
>
> Another option would be to occasionally dump information about phys
> and radios in all namespaces. While this is complicated it may be a
> little easier than having multiple hwsim instances because you can
> move threads in a single process into network namespaces. I.e. you
> can, for example, share an l_queue between temporary threads each of
> which moves into one network namespace, opens a netlink socket, dumps
> the phy information and exits. Or you can keep the threads alive to
> track netlink events but locking becomes more complicated.
Its good to hear from you :)
Yeah that could work too. I just already have this concept working for
other purposes, where the instances don't share the same kernel. So it
makes some sense to upstream this and have the communication be
generalized to an FD that could be Unix, UDP, TCP, etc. It may not be
useful to test-runner, but since I used hwsim as a base for it I feel
like it should be upstreamed if possible. I'll be sharing this soon.
For now, and purely test-runner purposes, watching for ADD/DEL_MAC_ADDR
events seems to be enough.
>
> Best regards
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-06-28 23:28 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-04 21:52 [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs James Prestwood
2023-05-04 21:52 ` [PATCH 2/3] test-runner: allow hwsim in namespaces James Prestwood
2023-05-04 21:52 ` [PATCH 3/3] test-runner: fix __str__ for namespace processes James Prestwood
2023-05-07 23:21 ` [PATCH 1/3] hwsim: remove 'optimization' sending to only known MACs Denis Kenzior
2023-05-08 13:43 ` James Prestwood
2023-05-08 18:05 ` Denis Kenzior
2023-05-08 18:55 ` James Prestwood
2023-05-08 19:00 ` Denis Kenzior
2023-05-08 19:03 ` James Prestwood
2023-05-08 19:01 ` Denis Kenzior
2023-06-21 21:05 ` James Prestwood
2023-06-27 2:31 ` Denis Kenzior
2023-06-27 15:15 ` James Prestwood
2023-06-27 18:00 ` Denis Kenzior
2023-06-27 18:56 ` James Prestwood
2023-06-27 19:23 ` Denis Kenzior
2023-06-27 20:09 ` James Prestwood
2023-06-28 14:49 ` Denis Kenzior
2023-06-28 15:33 ` James Prestwood
2023-06-28 15:40 ` Denis Kenzior
2023-06-28 16:14 ` James Prestwood
2023-06-28 16:25 ` Denis Kenzior
2023-06-28 16:47 ` James Prestwood
2023-06-28 16:57 ` Denis Kenzior
2023-06-28 17:22 ` James Prestwood
2023-06-28 23:19 ` Andrew Zaborowski
2023-06-28 23:28 ` James Prestwood
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.