[0/3] Mesh Fast xmit support

Message

Sriram R June 10, 2022, 10:40 a.m. UTC

Currently Fast xmit is supported for AP, STA and other device types where
the destination doesn't change for the lifetime of its association by
caching the static parts of the header that can be reused directly for
every Tx such as addresses and updates the mutable header fields such as
PN. This technique is not directly applicable for a Mesh device type
due to the dynamic nature of the topology and protocol. The header is
built based on the destination mesh device which is proxying a certain
external device and based on the Mesh destination the next hop changes.
And the RA/A1 which is the next hop for reaching the destination can
vary during runtime as per the best route based on airtime.  To
accommodate these changes and to come up with a solution to avoid
overhead during header generation, the headers comprising the MAC, Mesh
and LLC part are cached whenever data for a certain external destination
is sent. This cached header is reused every time a data is sent to that
external destination.

To ensure the changes in network are reflected in these cached headers,
the Mesh Proxy path table and Mesh path table changes are monitored
and corresponding headers are updated or flushed as applicable so that
the header used for a frame towards a certain destination is valid.

Old headers are flushed by the mesh housekeeping timers and based on the
cache size.

Only 6addr frame headers are cached currently.

Tested with ath11k driver.

Sriram R (3):
  cfg80211: increase mesh config attribute bitmask size
  cfg80211: Add provision for changing mesh header cache size
  mac80211: Mesh Fast xmit support

 include/net/cfg80211.h        |   5 +-
 include/uapi/linux/nl80211.h  |   4 +
 net/mac80211/cfg.c            |   6 +-
 net/mac80211/debugfs_netdev.c |   3 +
 net/mac80211/ieee80211_i.h    |  20 +++
 net/mac80211/mesh.c           |   2 +
 net/mac80211/mesh.h           |  45 +++++
 net/mac80211/mesh_hwmp.c      |   8 +-
 net/mac80211/mesh_pathtbl.c   | 396 ++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/rx.c             |   9 +-
 net/mac80211/tx.c             |  90 ++++++++++
 net/wireless/mesh.c           |   3 +
 net/wireless/nl80211.c        |  12 +-
 net/wireless/rdev-ops.h       |   2 +-
 net/wireless/trace.h          |   6 +-
 15 files changed, 596 insertions(+), 15 deletions(-)

Comments

Johannes Berg July 1, 2022, 8:48 a.m. UTC | #1

> 
> Sriram R (3):
>   cfg80211: increase mesh config attribute bitmask size
>   cfg80211: Add provision for changing mesh header cache size
> 

Is there really that much point in making that configurable? I have no
idea how a user could possibly set this to a reasonable value?

Maybe it would make more sense to auto-size it somehow depending on
memory? Or just pick a reasonable upper bound and leave it at that?

johannes

Sriram R July 1, 2022, 9:21 a.m. UTC | #2

>-----Original Message-----
>From: Johannes Berg <[e-mail]>
>Sent: Friday, July 1, 2022 2:19 PM
>To: Sriram R (QUIC) <[e-mail]>
>Cc: [e-mail]
>Subject: Re: [PATCH 0/3] Mesh Fast xmit support
>
>WARNING: This email originated from outside of Qualcomm. Please be wary of
>any links or attachments, and do not enable macros.
>
>>
>> Sriram R (3):
>>   cfg80211: increase mesh config attribute bitmask size
>>   cfg80211: Add provision for changing mesh header cache size
>>
>
>Is there really that much point in making that configurable? I have no idea how a
>user could possibly set this to a reasonable value?
Hi Johannes,
  Initially it was set it to a default size of 50 when RFC was sent. There was a suggestion to
make it configurable where users could configure this cache size proportional
to the required/anticipated network capacity.
>
>Maybe it would make more sense to auto-size it somehow depending on
>memory? Or just pick a reasonable upper bound and leave it at that?
>
Right, setting a good upper bound should be a relatively easy option, if we
don’t need this to be configurable.
Thanks,
Sriram.R

Johannes Berg July 1, 2022, 9:27 a.m. UTC | #3

>   Initially it was set it to a default size of 50 when RFC was sent.
> There was a suggestion to
> make it configurable where users could configure this cache size
> proportional to the required/anticipated network capacity.

Oh, right, I missed that this was in the discussion earlier.

The question is what are you afraid of? I mean, even setting it to 500
wouldn't be a huge amount of memory use (~50k), and probably mostly
sufficient regardless of the network? And if you never see all those
nodes, then it wouldn't use all that memory either.

Timing out old entries will also keep memory usage down.

So are you worried about worst-case behaviour in attacks, e.g. somebody
attempting to join the mesh? But then if you're worried about that I
guess you have bigger problems (and should be using secure mesh), such
as the number of station entries?

Or an attacker mutating their Ethernet address behind some gateway? But
they still need to convince the station to even want to send traffic
there...

But even then, setting a much higher limit than 50 should cope with
these cases, while giving enough breathing room for the real usage?

johannes

Sriram R July 1, 2022, 9:58 a.m. UTC | #4

>-----Original Message-----
>From: Johannes Berg <[e-mail]>
>Sent: Friday, July 1, 2022 2:57 PM
>To: Sriram R (QUIC) <[e-mail]>; [e-mail]
>Cc: [e-mail]
>Subject: Re: [PATCH 0/3] Mesh Fast xmit support
>
>WARNING: This email originated from outside of Qualcomm. Please be wary of
>any links or attachments, and do not enable macros.
>
>>   Initially it was set it to a default size of 50 when RFC was sent.
>> There was a suggestion to
>> make it configurable where users could configure this cache size
>> proportional to the required/anticipated network capacity.
>
>Oh, right, I missed that this was in the discussion earlier.
>
>The question is what are you afraid of? I mean, even setting it to 500 wouldn't
>be a huge amount of memory use (~50k), and probably mostly sufficient
>regardless of the network? And if you never see all those nodes, then it wouldn't
>use all that memory either.
>
>Timing out old entries will also keep memory usage down.
>
>So are you worried about worst-case behaviour in attacks, e.g. somebody
>attempting to join the mesh? But then if you're worried about that I guess you
>have bigger problems (and should be using secure mesh), such as the number of
>station entries?
>
>Or an attacker mutating their Ethernet address behind some gateway? But they
>still need to convince the station to even want to send traffic there...
>
>But even then, setting a much higher limit than 50 should cope with these cases,
>while giving enough breathing room for the real usage?
>
Hi Johannes,

   The only concern/reason is to not silently increase the memory requirement of Mesh
support with this patch.  So was skeptical on having a higher cache size(like 250 or 500 max).
Hence had a value of 50 and left the configuration part for devices which needed higher
cache. 
But as you mentioned, this is only runtime max memory and not default.
 So we should be fine to set some high limit, If above is not a concern could we stick to
an upper limit of ~150-200 ?

Apart from that, though the points you mentioned are quite possible, the cache
Management logic will ensure to cleanup stale entries and in worst case will
use regular header generation process if cache is full. So I feel that should ensure
things work as normal even under attack.

Thanks,
Sriram.R

Johannes Berg July 1, 2022, 9:59 a.m. UTC | #5

Hi,

>    The only concern/reason is to not silently increase the memory
> requirement of Mesh
> support with this patch.

OK.

>   So was skeptical on having a higher cache size(like 250 or 500 max).
> Hence had a value of 50 and left the configuration part for devices
> which needed higher
> cache. 
> But as you mentioned, this is only runtime max memory and not default.
>  So we should be fine to set some high limit, If above is not a
> concern could we stick to
> an upper limit of ~150-200 ?

Right, I'm fine with that. I was just throwing out 500 as a random
number to show that it's not really a huge memory requirement.

> Apart from that, though the points you mentioned are quite possible,
> the cache
> Management logic will ensure to cleanup stale entries and in worst
> case will
> use regular header generation process if cache is full. So I feel that
> should ensure
> things work as normal even under attack.

Right.

johannes

Johannes Berg July 1, 2022, 10 a.m. UTC | #6

On Fri, 2022-07-01 at 11:59 +0200, Johannes Berg wrote:
> 
> >   So was skeptical on having a higher cache size(like 250 or 500 max).
> > Hence had a value of 50 and left the configuration part for devices
> > which needed higher
> > cache. 
> > But as you mentioned, this is only runtime max memory and not default.
> >  So we should be fine to set some high limit, If above is not a
> > concern could we stick to
> > an upper limit of ~150-200 ?
> 
> Right, I'm fine with that. I was just throwing out 500 as a random
> number to show that it's not really a huge memory requirement.
> 

But maybe Felix wants to comment? Felix?

johannes

Sriram R July 15, 2022, 2:16 a.m. UTC | #7

>-----Original Message-----
>From: Johannes Berg <[e-mail]>
>Sent: Friday, July 1, 2022 3:30 PM
>To: Sriram R (QUIC) <[e-mail]>; [e-mail]
>Cc: [e-mail]
>Subject: Re: [PATCH 0/3] Mesh Fast xmit support
>
>WARNING: This email originated from outside of Qualcomm. Please be wary of
>any links or attachments, and do not enable macros.
>
>On Fri, 2022-07-01 at 11:59 +0200, Johannes Berg wrote:
>>
>> >   So was skeptical on having a higher cache size(like 250 or 500 max).
>> > Hence had a value of 50 and left the configuration part for devices
>> > which needed higher cache.
>> > But as you mentioned, this is only runtime max memory and not default.
>> >  So we should be fine to set some high limit, If above is not a
>> > concern could we stick to an upper limit of ~150-200 ?
>>
>> Right, I'm fine with that. I was just throwing out 500 as a random
>> number to show that it's not really a huge memory requirement.
>>
>
>But maybe Felix wants to comment? Felix?
Hi Felix,

  Could you kindly share your comments on this.

Thanks,
Sriram.R

Felix Fietkau July 17, 2022, 5:02 a.m. UTC | #8

On 15.07.22 04:16, Sriram R (QUIC) wrote:
>>-----Original Message-----
>>From: Johannes Berg <[e-mail]>
>>Sent: Friday, July 1, 2022 3:30 PM
>>To: Sriram R (QUIC) <[e-mail]>; [e-mail]
>>Cc: [e-mail]
>>Subject: Re: [PATCH 0/3] Mesh Fast xmit support
>>
>>WARNING: This email originated from outside of Qualcomm. Please be wary of
>>any links or attachments, and do not enable macros.
>>
>>On Fri, 2022-07-01 at 11:59 +0200, Johannes Berg wrote:
>>>
>>> >   So was skeptical on having a higher cache size(like 250 or 500 max).
>>> > Hence had a value of 50 and left the configuration part for devices
>>> > which needed higher cache.
>>> > But as you mentioned, this is only runtime max memory and not default.
>>> >  So we should be fine to set some high limit, If above is not a
>>> > concern could we stick to an upper limit of ~150-200 ?
>>>
>>> Right, I'm fine with that. I was just throwing out 500 as a random
>>> number to show that it's not really a huge memory requirement.
>>>
>>
>>But maybe Felix wants to comment? Felix?
> Hi Felix,
> 
>    Could you kindly share your comments on this.
I agree with making it big enough so that almost nobody has to tune it. 
I think 512 would be a reasonable default.
By the way, if I'm counting correctly, you might be able to reduce the 
size of the cache entries a bit by moving the 'key' field below the 
'band' field, getting rid of some padding.

- Felix

Sriram R July 17, 2022, 7:16 a.m. UTC | #9

>>>> >   So was skeptical on having a higher cache size(like 250 or 500 max).
>>>> > Hence had a value of 50 and left the configuration part for
>>>> > devices which needed higher cache.
>>>> > But as you mentioned, this is only runtime max memory and not default.
>>>> >  So we should be fine to set some high limit, If above is not a
>>>> > concern could we stick to an upper limit of ~150-200 ?
>>>>
>>>> Right, I'm fine with that. I was just throwing out 500 as a random
>>>> number to show that it's not really a huge memory requirement.
>>>>
>>>
>>>But maybe Felix wants to comment? Felix?
>> Hi Felix,
>>
>>    Could you kindly share your comments on this.
>I agree with making it big enough so that almost nobody has to tune it.
>I think 512 would be a reasonable default.
Sure.
>By the way, if I'm counting correctly, you might be able to reduce the size of the
>cache entries a bit by moving the 'key' field below the 'band' field, getting rid of
>some padding.
Oh okay, Thanks for checking, let me revisit this packing.

Regards,
Sriram.R

>
>- Felix