* Re: [PATCH v2 0/5] net: phy: mscc: add support for VSC8584 and VSC8574 Microsemi quad-port PHYs
From: David Miller @ 2018-10-05 21:42 UTC (permalink / raw)
To: quentin.schulz
Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, andrew, f.fainelli, allan.nielsen, linux-mips,
devicetree, linux-kernel, netdev, thomas.petazzoni,
antoine.tenart
In-Reply-To: <20181004131710.14978-1-quentin.schulz@bootlin.com>
From: Quentin Schulz <quentin.schulz@bootlin.com>
Date: Thu, 4 Oct 2018 15:17:05 +0200
> I suggest patches 1 to 3 go through net tree and patches 4 and 5 go
> through MIPS tree. Patches going through net tree and those going through
> MIPS tree do not depend on one another.
Sounds like a good plan but patches 1-3 do not apply to net-next, please
respin.
Thank you.
^ permalink raw reply
* [PATCH net-next] net: dsa: mc88e6xxx: Fix 88E6141/6341 2500mbps SERDES speed
From: Marek Behún @ 2018-10-05 14:42 UTC (permalink / raw)
To: netdev; +Cc: Andrew Lunn, Florian Fainelli, David S . Miller, Marek Behún
The port_set_speed method for the Topaz family must not be the same
as for Peridot family, since on Topaz port 5 is the SERDES port and it
can be set to 2500mbps speed mode.
This patch adds a new method for the Topaz family, allowing the alt_bit
mode only for port 0 and the 2500 mbps mode for port 5.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
drivers/net/dsa/mv88e6xxx/chip.c | 4 ++--
drivers/net/dsa/mv88e6xxx/port.c | 25 +++++++++++++++++++++++--
drivers/net/dsa/mv88e6xxx/port.h | 1 +
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 78ce820b5257..e05d4eddc935 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2907,7 +2907,7 @@ static const struct mv88e6xxx_ops mv88e6141_ops = {
.port_set_link = mv88e6xxx_port_set_link,
.port_set_duplex = mv88e6xxx_port_set_duplex,
.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
- .port_set_speed = mv88e6390_port_set_speed,
+ .port_set_speed = mv88e6341_port_set_speed,
.port_tag_remap = mv88e6095_port_tag_remap,
.port_set_frame_mode = mv88e6351_port_set_frame_mode,
.port_set_egress_floods = mv88e6352_port_set_egress_floods,
@@ -3528,7 +3528,7 @@ static const struct mv88e6xxx_ops mv88e6341_ops = {
.port_set_link = mv88e6xxx_port_set_link,
.port_set_duplex = mv88e6xxx_port_set_duplex,
.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
- .port_set_speed = mv88e6390_port_set_speed,
+ .port_set_speed = mv88e6341_port_set_speed,
.port_tag_remap = mv88e6095_port_tag_remap,
.port_set_frame_mode = mv88e6351_port_set_frame_mode,
.port_set_egress_floods = mv88e6352_port_set_egress_floods,
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 92945841c8e8..cd7db60a508b 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -228,8 +228,11 @@ static int mv88e6xxx_port_set_speed(struct mv88e6xxx_chip *chip, int port,
ctrl = MV88E6XXX_PORT_MAC_CTL_SPEED_1000;
break;
case 2500:
- ctrl = MV88E6390_PORT_MAC_CTL_SPEED_10000 |
- MV88E6390_PORT_MAC_CTL_ALTSPEED;
+ if (alt_bit)
+ ctrl = MV88E6390_PORT_MAC_CTL_SPEED_10000 |
+ MV88E6390_PORT_MAC_CTL_ALTSPEED;
+ else
+ ctrl = MV88E6390_PORT_MAC_CTL_SPEED_10000;
break;
case 10000:
/* all bits set, fall through... */
@@ -291,6 +294,24 @@ int mv88e6185_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed)
return mv88e6xxx_port_set_speed(chip, port, speed, false, false);
}
+/* Support 10, 100, 200, 1000, 2500 Mbps (e.g. 88E6341) */
+int mv88e6341_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed)
+{
+ if (speed == SPEED_MAX)
+ speed = port < 5 ? 1000 : 2500;
+
+ if (speed > 2500)
+ return -EOPNOTSUPP;
+
+ if (speed == 200 && port != 0)
+ return -EOPNOTSUPP;
+
+ if (speed == 2500 && port < 5)
+ return -EOPNOTSUPP;
+
+ return mv88e6xxx_port_set_speed(chip, port, speed, !port, true);
+}
+
/* Support 10, 100, 200, 1000 Mbps (e.g. 88E6352 family) */
int mv88e6352_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed)
{
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index f32f56af8e35..36904c9bf955 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -269,6 +269,7 @@ int mv88e6xxx_port_set_duplex(struct mv88e6xxx_chip *chip, int port, int dup);
int mv88e6065_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed);
int mv88e6185_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed);
+int mv88e6341_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed);
int mv88e6352_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed);
int mv88e6390_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed);
int mv88e6390x_port_set_speed(struct mv88e6xxx_chip *chip, int port, int speed);
--
2.16.4
^ permalink raw reply related
* Re: [PATCH net-next RFC 0/8] udp and configurable gro
From: Willem de Bruijn @ 2018-10-05 14:41 UTC (permalink / raw)
To: Paolo Abeni
Cc: Network Development, steffen.klassert, David Miller,
Willem de Bruijn
In-Reply-To: <736e48946dee45db19c13c9d64f1aa21e0c8ef99.camel@redhat.com>
On Fri, Oct 5, 2018 at 9:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi all,
>
> On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > This is a *very rough* draft. Mainly for discussion while we also
> > look at another partially overlapping approach [1].
>
> I'm wondering how we go on from this ? I'm fine with either approaches.
Let me send the udp gro static_key patch. Then we don't need
the enable udp on demand logic (patch 2/4).
Your implementation of GRO is more fleshed out (patch 3/4) than
my quick hack. My only request would be to use a separate
UDP_GRO socket option instead of adding this to the existing
UDP_SEGMENT.
Sounds good?
> Also, I'm interested in [try to] enable GRO/GSO batching in the
> forwarding path, as you outlined initially in the GSO series
> submission. That should cover Steffen use-case, too, right?
Great. Indeed. Though there is some unresolved discussion on
one large gso skb vs frag list. There has been various concerns
around the use of frag lists for GSO in the past, and it does not
match h/w offload. So I think the answer would be the first unless
the second proves considerably faster (in which case it could also
be added later as optimization).
^ permalink raw reply
* Re: [PATCH net-next v4 00/11] mscc: ocelot: add support for SerDes muxing configuration
From: David Miller @ 2018-10-05 21:39 UTC (permalink / raw)
To: quentin.schulz
Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, kishon, andrew, f.fainelli, allan.nielsen,
linux-mips, devicetree, linux-kernel, netdev, thomas.petazzoni
In-Reply-To: <20181004122208.32272-1-quentin.schulz@bootlin.com>
From: Quentin Schulz <quentin.schulz@bootlin.com>
Date: Thu, 4 Oct 2018 14:21:57 +0200
> The Ocelot switch has currently an hardcoded SerDes muxing that suits only
> a particular use case. Any other board setup will fail to work.
>
> To prepare for upcoming boards' support that do not have the same muxing,
> create a PHY driver that will handle all possible cases.
>
> A SerDes can work in SGMII, QSGMII or PCIe and is also muxed to use a
> given port depending on the selected mode or board design.
>
> The SerDes configuration is in the middle of an address space (HSIO) that
> is used to configure some parts in the MAC controller driver, that is why
> we need to use a syscon so that we can write to the same address space from
> different drivers safely using regmap.
>
> This breaks backward compatibility but it's fine because there's only one
> board at the moment that is using what's modified in this patch series.
> This will break git bisect.
>
> Even though this patch series is about SerDes __muxing__ configuration, the
> DT node is named serdes for the simple reason that I couldn't find any
> mention to SerDes anywhere else from the address space handled by this
> driver.
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] atm: nicstar: Replace spin_is_locked() with spin_trylock()
From: David Miller @ 2018-10-05 21:32 UTC (permalink / raw)
To: ldr709; +Cc: linux-kernel, paulmck, 3chas3, linux-atm-general, netdev
In-Reply-To: <20181004074657.17597-1-ldr709@gmail.com>
From: Lance Roy <ldr709@gmail.com>
Date: Thu, 4 Oct 2018 00:46:57 -0700
> ns_poll() used spin_is_locked() + spin_lock() to get achieve the same
> thing as a spin_trylock(), so simplify it by using that instead. This is
> also a step towards possibly removing spin_is_locked().
>
> Signed-off-by: Lance Roy <ldr709@gmail.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 0/3] bpf: allow zero-initialising hash map seed
From: Jann Horn @ 2018-10-05 14:29 UTC (permalink / raw)
To: lmb; +Cc: Daniel Borkmann, Alexei Starovoitov, Network Development,
Linux API
In-Reply-To: <CACAyw99h58NX6tE1KrgoNEWwX4U8WW3JaRnJGJ0EGXEoeqeMDw@mail.gmail.com>
On Fri, Oct 5, 2018 at 4:27 PM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> On Mon, 1 Oct 2018 at 20:12, Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 10/01/2018 12:45 PM, Lorenz Bauer wrote:
> > > This patch set adds a new flag BPF_F_ZERO_SEED, which allows
> > > forcing the seed used by hash maps to zero. This makes
> > > it possible to write deterministic tests.
> > >
> > > Based on an off-list conversation with Alexei Starovoitov and
> > > Daniel Borkmann.
> > >
> > > Lorenz Bauer (3):
> > > bpf: allow zero-initializing hash map seed
> > > tools: sync linux/bpf.h
> > > tools: add selftest for BPF_F_ZERO_SEED
> > >
> > > include/uapi/linux/bpf.h | 2 +
> > > kernel/bpf/hashtab.c | 8 ++-
> > > tools/include/uapi/linux/bpf.h | 2 +
> > > tools/testing/selftests/bpf/test_maps.c | 67 +++++++++++++++++++++----
> > > 4 files changed, 66 insertions(+), 13 deletions(-)
> > >
> >
> > Please respin with proper SoB for each patch and non-empty commit
> > description.
>
> What does SoB mean? Point taken about the empty commit message.
SoB is the Signed-off-by line. See
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
.
^ permalink raw reply
* Re: [PATCH 1/3] bpf: allow zero-initializing hash map seed
From: Jann Horn @ 2018-10-05 14:27 UTC (permalink / raw)
To: lmb; +Cc: Alexei Starovoitov, Daniel Borkmann, Network Development,
Linux API
In-Reply-To: <CACAyw9-JcORMGDb3wm-E4VYmjVmvWqB5vVh+=qTvXmpFWye_QA@mail.gmail.com>
On Fri, Oct 5, 2018 at 4:21 PM Lorenz Bauer <lmb@cloudflare.com> wrote:
> On Fri, 5 Oct 2018 at 15:12, Jann Horn <jannh@google.com> wrote:
> > On Fri, Oct 5, 2018 at 9:42 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> > > On Tue, 2 Oct 2018 at 21:00, Jann Horn <jannh@google.com> wrote:
> > > > If this is for testing only, you can slap a capable(CAP_SYS_ADMIN)
> > > > check in here, right? I doubt it matters, but I don't really like
> > > > seeing something like this exposed to unprivileged userspace just
> > > > because you need it for kernel testing.
> > >
> > > That would mean all tests have to run as root / with CAP_SYS_ADMIN
> > > which isn't ideal.
> >
> > This patch basically means that it becomes easier for a local user to
> > construct a BPF hash table that has all of its values stuffed into a
> > single hash bucket, correct? Which makes it easier to create a BPF
> > program that generates unusually large RCU stalls by performing ~40000
> > BPF map lookups, each of which has to walk through the entire linked
> > list of the hash map bucket? I dislike exposing something like that to
> > unprivileged userspace.
>
> That's a good point, for which I don't have an answer. You could argue that
> this was the status quo until the seed was randomised, so it seems
> like this hasn't been a worry so far. Should it be going forward?
I don't think that local DoS bugs, or bugs that locally degrade
performance, are a big deal, but I also think that the kernel should
try to avoid having such issues.
> > And if you want to run the whole BPF test suite with all its tests,
> > don't you already need root privileges? Or is this a different test
> > suite?
>
> No, I'm thinking about third parties that want to test their own BPF.
Ah. That wasn't clear to me from your patch description.
Can you please describe exactly why something that is not a kernel
unit test needs deterministic BPF hash map behavior?
> If you enable unprivileged BPF you can use BPF_PROG_TEST_RUN to
> test your programs without root, if I'm not mistaken.
^ permalink raw reply
* Re: [PATCH 0/3] bpf: allow zero-initialising hash map seed
From: Lorenz Bauer @ 2018-10-05 14:27 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev, linux-api
In-Reply-To: <59ef80ab-4f28-5c75-c394-55fcfd9bc8ca@iogearbox.net>
On Mon, 1 Oct 2018 at 20:12, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 10/01/2018 12:45 PM, Lorenz Bauer wrote:
> > This patch set adds a new flag BPF_F_ZERO_SEED, which allows
> > forcing the seed used by hash maps to zero. This makes
> > it possible to write deterministic tests.
> >
> > Based on an off-list conversation with Alexei Starovoitov and
> > Daniel Borkmann.
> >
> > Lorenz Bauer (3):
> > bpf: allow zero-initializing hash map seed
> > tools: sync linux/bpf.h
> > tools: add selftest for BPF_F_ZERO_SEED
> >
> > include/uapi/linux/bpf.h | 2 +
> > kernel/bpf/hashtab.c | 8 ++-
> > tools/include/uapi/linux/bpf.h | 2 +
> > tools/testing/selftests/bpf/test_maps.c | 67 +++++++++++++++++++++----
> > 4 files changed, 66 insertions(+), 13 deletions(-)
> >
>
> Please respin with proper SoB for each patch and non-empty commit
> description.
What does SoB mean? Point taken about the empty commit message.
> I think patch 1 should also have a more elaborate
> commit description on the use case for BPF_F_ZERO_SEED, and I
This came out of the off-list discussion we had about map hash functions,
where Alexei expressed concern that your change to randomise the seed
might catch users off-guard. I personally don't have a use case, but decided
to tackle it since it seemed a simple-ish fix to get acquainted with
the code base.
Maybe this isn't needed after all?
> think also a better comment in the uapi header that this is only
> meant for testing and not production use.
Will do, if you decide that this is worth having in the first place.
>
> Thanks,
> Daniel
Lorenz
--
Lorenz Bauer | Systems Engineer
25 Lavington St., London SE1 0NZ
www.cloudflare.com
^ permalink raw reply
* Re: [PATCH] usbnet: smsc95xx: simplify tx_fixup code
From: David Miller @ 2018-10-05 21:24 UTC (permalink / raw)
To: ben.dooks
Cc: netdev, David.Laight, oneukum, linux-usb, linux-kernel,
linux-kernel
In-Reply-To: <20181002165602.21033-1-ben.dooks@codethink.co.uk>
From: Ben Dooks <ben.dooks@codethink.co.uk>
Date: Tue, 2 Oct 2018 17:56:02 +0100
> - memcpy(skb->data, &tx_cmd_a, 4);
> + ptr = skb_push(skb, 8);
> + tx_cmd_a = cpu_to_le32(tx_cmd_a);
> + tx_cmd_b = cpu_to_le32(tx_cmd_b);
> + memcpy(ptr, &tx_cmd_a, 4);
> + memcpy(ptr+4, &tx_cmd_b, 4);
Even a memcpy() through a void pointer does not guarantee that gcc will
not emit word sized loads and stores.
You must use the get_unaligned()/put_unaligned() facilities to do this
properly.
I also agree that making a proper type and structure instead of using
a void pointer would be better.
^ permalink raw reply
* Re: [PATCH 1/3] bpf: allow zero-initializing hash map seed
From: Lorenz Bauer @ 2018-10-05 14:21 UTC (permalink / raw)
To: jannh; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, linux-api
In-Reply-To: <CAG48ez3wwGgc40uAg5nq9dnC3qsAD04jQZTWtPi7jg_VDws-fg@mail.gmail.com>
On Fri, 5 Oct 2018 at 15:12, Jann Horn <jannh@google.com> wrote:
>
> On Fri, Oct 5, 2018 at 9:42 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> > On Tue, 2 Oct 2018 at 21:00, Jann Horn <jannh@google.com> wrote:
> > >
> > > If this is for testing only, you can slap a capable(CAP_SYS_ADMIN)
> > > check in here, right? I doubt it matters, but I don't really like
> > > seeing something like this exposed to unprivileged userspace just
> > > because you need it for kernel testing.
> >
> > That would mean all tests have to run as root / with CAP_SYS_ADMIN
> > which isn't ideal.
>
> This patch basically means that it becomes easier for a local user to
> construct a BPF hash table that has all of its values stuffed into a
> single hash bucket, correct? Which makes it easier to create a BPF
> program that generates unusually large RCU stalls by performing ~40000
> BPF map lookups, each of which has to walk through the entire linked
> list of the hash map bucket? I dislike exposing something like that to
> unprivileged userspace.
That's a good point, for which I don't have an answer. You could argue that
this was the status quo until the seed was randomised, so it seems
like this hasn't been a worry so far. Should it be going forward?
> And if you want to run the whole BPF test suite with all its tests,
> don't you already need root privileges? Or is this a different test
> suite?
No, I'm thinking about third parties that want to test their own BPF.
If you enable unprivileged BPF you can use BPF_PROG_TEST_RUN to
test your programs without root, if I'm not mistaken.
--
Lorenz Bauer | Systems Engineer
25 Lavington St., London SE1 0NZ
www.cloudflare.com
^ permalink raw reply
* Re: [PATCH 1/3] bpf: allow zero-initializing hash map seed
From: Jann Horn @ 2018-10-05 14:12 UTC (permalink / raw)
To: lmb; +Cc: Alexei Starovoitov, Daniel Borkmann, Network Development,
Linux API
In-Reply-To: <CACAyw99ar0Wokyg4jx8FPyr1Z=7Cf+=WVgF6sLL1FK9JtHROjw@mail.gmail.com>
On Fri, Oct 5, 2018 at 9:42 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> On Tue, 2 Oct 2018 at 21:00, Jann Horn <jannh@google.com> wrote:
> >
> > If this is for testing only, you can slap a capable(CAP_SYS_ADMIN)
> > check in here, right? I doubt it matters, but I don't really like
> > seeing something like this exposed to unprivileged userspace just
> > because you need it for kernel testing.
>
> That would mean all tests have to run as root / with CAP_SYS_ADMIN
> which isn't ideal.
This patch basically means that it becomes easier for a local user to
construct a BPF hash table that has all of its values stuffed into a
single hash bucket, correct? Which makes it easier to create a BPF
program that generates unusually large RCU stalls by performing ~40000
BPF map lookups, each of which has to walk through the entire linked
list of the hash map bucket? I dislike exposing something like that to
unprivileged userspace.
And if you want to run the whole BPF test suite with all its tests,
don't you already need root privileges? Or is this a different test
suite?
^ permalink raw reply
* Re: [PATCH net-next v3] wireless-drivers: rtnetlink wifi simulation device
From: Joel Fernandes @ 2018-10-05 21:08 UTC (permalink / raw)
To: Sergey Matyukevich
Cc: Cody Schuffelen, Johannes Berg, Kalle Valo, David S . Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org
In-Reply-To: <CAJWu+ooTRUxj2PAiUkgjTLb+VC9OoBpkj9-_ZoC-oCS3owc5Hg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Oct 5, 2018 at 2:05 PM, Joel Fernandes <joelaf-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, Oct 5, 2018 at 7:33 AM, Sergey Matyukevich
> <sergey.matyukevich.os-P/7pdk10T0iB+jHODAdFcQ@public.gmane.org> wrote:
>> Hi Cody,
>>
>>> drivers/net/wireless/Kconfig | 7 +
>>> drivers/net/wireless/Makefile | 2 +
>>> drivers/net/wireless/virt_wifi.c | 618 +++++++++++++++++++++++++++++++
>>> 3 files changed, 627 insertions(+)
>>> create mode 100644 drivers/net/wireless/virt_wifi.c
>>
>> I did a quick check of your patch using checkpatch kernel tool,
>> here is a summary of its output:
>>
>> $ ./scripts/checkpatch.pl --strict test.patch
>> ...
>> total: 165 errors, 428 warnings, 9 checks, 634 lines checked
>>
>> Most part of those complaints is about either whitespaces or code
>> idents. I am not sure whether this is a patch itself or email client.
>> So could you please take a look and run checkpatch on your side.
>>
>
> Yeah, it could be his email client, weird though because if I pull the
> patch from the kernel.org archive's mbox though, I don't get any
> errors except the MAINTAINERS file thing:
>
> wget https://lore.kernel.org/lkml/20181004195906.201895-1-schuffelen-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org/raw
> -O /tmp/tmp.patch
> ./scripts/checkpatch.pl --strict /tmp/tmp.patch
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #167:
> new file mode 100644
>
> total: 0 errors, 1 warnings, 0 checks, 634 lines checked
>
FWIW, the X-Mailer on the patch is: git-send-email 2.19.0.605.g01d371f741-goog
So I am guessing this is some issue with the way the patch was
generated, or something else. Cody, care to share the steps you used
to generate and send the patch?
- Joel
^ permalink raw reply
* Re: [PATCH net-next v3] wireless-drivers: rtnetlink wifi simulation device
From: Joel Fernandes @ 2018-10-05 21:05 UTC (permalink / raw)
To: Sergey Matyukevich
Cc: Cody Schuffelen, Johannes Berg, Kalle Valo, David S . Miller,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, kernel-team@android.com
In-Reply-To: <20181005143323.ezyd2x6x5ymlb7rg@bars>
On Fri, Oct 5, 2018 at 7:33 AM, Sergey Matyukevich
<sergey.matyukevich.os@quantenna.com> wrote:
> Hi Cody,
>
>> drivers/net/wireless/Kconfig | 7 +
>> drivers/net/wireless/Makefile | 2 +
>> drivers/net/wireless/virt_wifi.c | 618 +++++++++++++++++++++++++++++++
>> 3 files changed, 627 insertions(+)
>> create mode 100644 drivers/net/wireless/virt_wifi.c
>
> I did a quick check of your patch using checkpatch kernel tool,
> here is a summary of its output:
>
> $ ./scripts/checkpatch.pl --strict test.patch
> ...
> total: 165 errors, 428 warnings, 9 checks, 634 lines checked
>
> Most part of those complaints is about either whitespaces or code
> idents. I am not sure whether this is a patch itself or email client.
> So could you please take a look and run checkpatch on your side.
>
Yeah, it could be his email client, weird though because if I pull the
patch from the kernel.org archive's mbox though, I don't get any
errors except the MAINTAINERS file thing:
wget https://lore.kernel.org/lkml/20181004195906.201895-1-schuffelen@google.com/raw
-O /tmp/tmp.patch
./scripts/checkpatch.pl --strict /tmp/tmp.patch
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#167:
new file mode 100644
total: 0 errors, 1 warnings, 0 checks, 634 lines checked
- Joel
^ permalink raw reply
* Re: [PATCH net-next RFC 0/8] udp and configurable gro
From: Paolo Abeni @ 2018-10-05 13:53 UTC (permalink / raw)
To: Willem de Bruijn, netdev; +Cc: steffen.klassert, davem, Willem de Bruijn
In-Reply-To: <20180914175941.213950-1-willemdebruijn.kernel@gmail.com>
Hi all,
On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> This is a *very rough* draft. Mainly for discussion while we also
> look at another partially overlapping approach [1].
I'm wondering how we go on from this ? I'm fine with either approaches.
Also, I'm interested in [try to] enable GRO/GSO batching in the
forwarding path, as you outlined initially in the GSO series
submission. That should cover Steffen use-case, too, right?
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
From: Andrew Lunn @ 2018-10-05 13:53 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Florian Fainelli, netdev@vger.kernel.org, Alexandru Marginean
In-Reply-To: <HE1PR0402MB3323DB8B3C52F4C14D3BC46DF1EB0@HE1PR0402MB3323.eurprd04.prod.outlook.com>
> > > + ret = regmap_update_bits_check(s->regmap,
> > > + s->mux_reg,
> > > + s->mask,
> > > + desired_child,
> > > + &change);
> >
> > When getting the mask from DT, you use be32_to_cpup().
> > When testing the reg value against the mask, you use be32_to_cpup().
> > Here you do not use be32_to_cpup()?
>
> Can you please tell me for which variable you mean I should use be32_to_cpup?
> I use be32_to_cpup when reading device tree entries.
> After being read, their values are stored in structure. After that no need to do be32_to_cpup
desired_child is read from DT by the mdio-mux core.
I'm just wondering why any of this be32_to_cpup() is needed. Why not
just use of_property_read_u32()?
Andrew
^ permalink raw reply
* [PATCH] mac80211_hwsim: fix module init error paths for netlink
From: Alexey Khoroshilov @ 2018-10-05 20:22 UTC (permalink / raw)
To: Johannes Berg
Cc: Alexey Khoroshilov, Kalle Valo, linux-wireless, netdev,
linux-kernel, ldv-project
There is no unregister netlink notifier and family on error paths
in init_mac80211_hwsim(). Also there is an error path where
hwsim_class is not destroyed.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Fixes: 62759361eb49 ("mac80211-hwsim: Provide multicast event for HWSIM_CMD_NEW_RADIO")
---
drivers/net/wireless/mac80211_hwsim.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 07442ada6dd0..6532cb25a333 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3712,16 +3712,16 @@ static int __init init_mac80211_hwsim(void)
if (err)
goto out_unregister_pernet;
+ err = hwsim_init_netlink();
+ if (err)
+ goto out_unregister_driver;
+
hwsim_class = class_create(THIS_MODULE, "mac80211_hwsim");
if (IS_ERR(hwsim_class)) {
err = PTR_ERR(hwsim_class);
- goto out_unregister_driver;
+ goto out_exit_netlink;
}
- err = hwsim_init_netlink();
- if (err < 0)
- goto out_unregister_driver;
-
for (i = 0; i < radios; i++) {
struct hwsim_new_radio_params param = { 0 };
@@ -3827,6 +3827,8 @@ static int __init init_mac80211_hwsim(void)
free_netdev(hwsim_mon);
out_free_radios:
mac80211_hwsim_free();
+out_exit_netlink:
+ hwsim_exit_netlink();
out_unregister_driver:
platform_driver_unregister(&mac80211_hwsim_driver);
out_unregister_pernet:
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH] skb: Define NET_IP_ALIGN based on CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
From: Will Deacon @ 2018-10-05 13:16 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Ben Hutchings, Russell King, Catalin Marinas,
<netdev@vger.kernel.org>, linux-kernel, linux-s390,
Ben Dooks, linux-arm-kernel
In-Reply-To: <CAKv+Gu9MBJ0w+23XMg+w_EYEf0Hx8dkW-w-rf4Bzu_c3GN_YiQ@mail.gmail.com>
On Thu, Oct 04, 2018 at 07:43:59PM +0200, Ard Biesheuvel wrote:
> (+ Arnd, Russell, Catalin, Will)
>
> On 4 October 2018 at 19:36, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > NET_IP_ALIGN is supposed to be defined as 0 if DMA writes to an
> > unaligned buffer would be more expensive than CPU access to unaligned
> > header fields, and otherwise defined as 2.
> >
> > Currently only ppc64 and x86 configurations define it to be 0.
> > However several other architectures (conditionally) define
> > CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS, which seems to imply that
> > NET_IP_ALIGN should be 0.
> >
> > Remove the overriding definitions for ppc64 and x86 and define
> > NET_IP_ALIGN solely based on CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
> >
> > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
>
> While this makes sense for arm64, I don't think it is appropriate for
> ARM per se.
Agreed that this makes sense for arm64, and I'd be happy to take a patch
defining it as 0 there.
Will
^ permalink raw reply
* Re: [PATCH net-next,v2] IPv6 ifstats separation
From: Eric Dumazet @ 2018-10-05 13:13 UTC (permalink / raw)
To: Stephen Suryaputra, eric.dumazet; +Cc: netdev
In-Reply-To: <CAHapkUhWNyS0SGQmHh11SNKKtP7c2V9ct6R-9EZO01QXdLE1ug@mail.gmail.com>
On 10/05/2018 06:00 AM, Stephen Suryaputra wrote:
> On Thu, Oct 4, 2018 at 4:42 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> How have you decided some counters can be 'slow' and other 'fast' ?
>>
>> I can tell you I see many ultra-fast candidates in your 'slow' list :/
>
> Based on what others have categorized based on what's in the code and
> IMHO they make sense:
Well, you better test, because you missed a few counters that are hit hard in the fast
path for normal (non DDOS) packets.
>
> enum
> {
> IPSTATS_MIB_NUM = 0,
> /* frequently written fields in fast path, kept in same cache line */
> IPSTATS_MIB_INPKTS, /* InReceives */
> IPSTATS_MIB_INOCTETS, /* InOctets */
> IPSTATS_MIB_INDELIVERS, /* InDelivers */
> IPSTATS_MIB_OUTFORWDATAGRAMS, /* OutForwDatagrams */
> IPSTATS_MIB_OUTPKTS, /* OutRequests */
> IPSTATS_MIB_OUTOCTETS, /* OutOctets */
> /* other fields */
> IPSTATS_MIB_INHDRERRORS, /* InHdrErrors */
> ...
> __IPSTATS_MIB_MAX
> };
>
>>
>> Also think about DDOS.
>>
>> After your patch, all these 'wrong packets' will incur an expensive
>> operation on a shared and highly contented cache line,
>> effectively making the attack easier to conduct.
>>
>
> I agree about it is becoming more expensive to hit the slow counters
> due to the check whether they are enabled or not.
What do you mean ?
The real cost is having dozens of cpus updating the same cache lines if the SNMP counter
is an atomic instead of per-cpu counters.
Make sure to test this on a configuration with 16 (or more) RX queues,
and cpus handling NIC IRQS spread on multiple NUMA nodes.
^ permalink raw reply
* [PATCH net-next] bnxt_en: Remove unnecessary unsigned integer comparison and initialize variable
From: Gustavo A. R. Silva @ 2018-10-05 20:12 UTC (permalink / raw)
To: Michael Chan, David S. Miller; +Cc: netdev, linux-kernel, Gustavo A. R. Silva
There is no need to compare *val.vu32* with < 0 because
such variable is of type u32 (32 bits, unsigned), making it
impossible to hold a negative value. Fix this by removing
such comparison.
Also, initialize variable *max_val* to -1, just in case
it is not initialized to either BNXT_MSIX_VEC_MAX or
BNXT_MSIX_VEC_MIN_MAX before using it in a comparison
with val.vu32 at line 159:
if (val.vu32 > max_val)
Addresses-Coverity-ID: 1473915 ("Unsigned compared against 0")
Addresses-Coverity-ID: 1473920 ("Uninitialized scalar variable")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 8a10e01..140dbd6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -148,7 +148,7 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
union devlink_param_value val,
struct netlink_ext_ack *extack)
{
- int max_val;
+ int max_val = -1;
if (id == DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX)
max_val = BNXT_MSIX_VEC_MAX;
@@ -156,7 +156,7 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
if (id == DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN)
max_val = BNXT_MSIX_VEC_MIN_MAX;
- if (val.vu32 < 0 || val.vu32 > max_val) {
+ if (val.vu32 > max_val) {
NL_SET_ERR_MSG_MOD(extack, "MSIX value is exceeding the range");
return -EINVAL;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 0/5] net: Consolidate metrics handling for ipv4 and ipv6
From: Eric Dumazet @ 2018-10-05 13:08 UTC (permalink / raw)
To: Eric Dumazet, David Miller, dsahern
Cc: netdev, weiwan, sd, xiyou.wangcong, dsahern
In-Reply-To: <2b4d849f-9bd3-28aa-7a1c-ad61ab584041@gmail.com>
On 10/05/2018 05:17 AM, Eric Dumazet wrote:
>
>
> On 10/04/2018 09:55 PM, David Miller wrote:
>> From: David Ahern <dsahern@kernel.org>
>> Date: Thu, 4 Oct 2018 20:07:50 -0700
>>
>>> From: David Ahern <dsahern@gmail.com>
>>>
>>> As part of the IPv6 fib info refactoring, the intent was to make metrics
>>> handling for ipv6 identical to ipv4. One oversight in ip6_dst_destroy
>>> led to confusion and a couple of incomplete attempts at finding and
>>> fixing the resulting memory leak which was ultimately resolved by
>>> ce7ea4af0838 ("ipv6: fix memory leak on dst->_metrics").
>>>
>>> Refactor metrics hanlding make the code really identical for v4 and v6,
>>> and add a few test cases.
>>
>> Looks nice, series applied, thanks David.
>>
>
> Does not look well tested and reviewed to me.
For some reason I have not received the patch series in my inbox, I only got
your "series applied" message.
Commit 767a2217533fed6 ("net: common metrics init helper for FIB entries")
is not correct because we need to better deal with error paths.
I will submit this more formally when I can reach my workstation in a few minutes :
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6c1d817151cae45421dc976c5ea082b4115650be..74d97addf1af20dda0c2b6a2018e88696f9f7d5a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2976,6 +2976,8 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len);
if (IS_ERR(rt->fib6_metrics)) {
err = PTR_ERR(rt->fib6_metrics);
+ /* Do not leave garbage there. */
+ rt->fib6_metrics = (struct dst_metrics *)&dst_default_metrics;
goto out;
}
^ permalink raw reply related
* Re: [PATCH net-next,v2] IPv6 ifstats separation
From: Stephen Suryaputra @ 2018-10-05 13:00 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <cdf02f85-57ea-3371-cec3-ec155964f4d0@gmail.com>
On Thu, Oct 4, 2018 at 4:42 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> How have you decided some counters can be 'slow' and other 'fast' ?
>
> I can tell you I see many ultra-fast candidates in your 'slow' list :/
Based on what others have categorized based on what's in the code and
IMHO they make sense:
enum
{
IPSTATS_MIB_NUM = 0,
/* frequently written fields in fast path, kept in same cache line */
IPSTATS_MIB_INPKTS, /* InReceives */
IPSTATS_MIB_INOCTETS, /* InOctets */
IPSTATS_MIB_INDELIVERS, /* InDelivers */
IPSTATS_MIB_OUTFORWDATAGRAMS, /* OutForwDatagrams */
IPSTATS_MIB_OUTPKTS, /* OutRequests */
IPSTATS_MIB_OUTOCTETS, /* OutOctets */
/* other fields */
IPSTATS_MIB_INHDRERRORS, /* InHdrErrors */
...
__IPSTATS_MIB_MAX
};
>
> Also think about DDOS.
>
> After your patch, all these 'wrong packets' will incur an expensive
> operation on a shared and highly contented cache line,
> effectively making the attack easier to conduct.
>
I agree about it is becoming more expensive to hit the slow counters
due to the check whether they are enabled or not.
Do you think it should just be enabled always? Then the cost to hit
the slow counters is the same as what it is right now.
Stephen.
^ permalink raw reply
* Re: [PATCH net-next 0/5] net: Consolidate metrics handling for ipv4 and ipv6
From: Eric Dumazet @ 2018-10-05 12:17 UTC (permalink / raw)
To: David Miller, dsahern; +Cc: netdev, weiwan, sd, xiyou.wangcong, dsahern
In-Reply-To: <20181004.215507.1973705600397352485.davem@davemloft.net>
On 10/04/2018 09:55 PM, David Miller wrote:
> From: David Ahern <dsahern@kernel.org>
> Date: Thu, 4 Oct 2018 20:07:50 -0700
>
>> From: David Ahern <dsahern@gmail.com>
>>
>> As part of the IPv6 fib info refactoring, the intent was to make metrics
>> handling for ipv6 identical to ipv4. One oversight in ip6_dst_destroy
>> led to confusion and a couple of incomplete attempts at finding and
>> fixing the resulting memory leak which was ultimately resolved by
>> ce7ea4af0838 ("ipv6: fix memory leak on dst->_metrics").
>>
>> Refactor metrics hanlding make the code really identical for v4 and v6,
>> and add a few test cases.
>
> Looks nice, series applied, thanks David.
>
Does not look well tested and reviewed to me.
^ permalink raw reply
* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
From: Gustavo A. R. Silva @ 2018-10-05 19:15 UTC (permalink / raw)
To: Ben Greear, Kalle Valo, David S. Miller
Cc: ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <dd75a8aa-b87c-e325-cc1d-a0efe3664a32-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
>
>
> On 10/5/18 9:09 PM, Ben Greear wrote:
>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>> Currently, the error handling for the call to function
>>> ath10k_get_legacy_rate_idx() doesn't work because
>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>> makes it impossible for it to hold a value less
>>> than 0.
>>>
>>> Fix this by changing the type of variable *rate_idx*
>>> to s8 (8 bits, signed).
>>
>> There are more than 127 rates, are you sure this is doing
>> what you want?
>>
>
> Based on the following function, rate_idx can only hold values from 0 to 11
>
... and of course -EINVAL too
> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
> {
> static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
> 18, 24, 36, 48, 54};
> int i;
>
> for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
> if (rate == legacy_rates[i])
> return i;
> }
>
> ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
> return -EINVAL;
> }
>
> Thanks
> --
> Gustavo
>
>> Thanks,
>> Ben
>>
>>>
>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>>> Signed-off-by: Gustavo A. R. Silva <gustavo-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
>>> ---
>>> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index f240525..edd0e74 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>> struct ath10k_per_peer_tx_stats *peer_stats)
>>> {
>>> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>> - u8 rate = 0, rate_idx = 0, sgi;
>>> + u8 rate = 0, sgi;
>>> + s8 rate_idx = 0;
>>> struct rate_info txrate;
>>>
>>> lockdep_assert_held(&ar->data_lock);
>>>
>>
>>
^ permalink raw reply
* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
From: Gustavo A. R. Silva @ 2018-10-05 19:14 UTC (permalink / raw)
To: Ben Greear, Kalle Valo, David S. Miller
Cc: ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <3a55bec6-d20d-f500-e741-b228a86b7117@candelatech.com>
On 10/5/18 9:09 PM, Ben Greear wrote:
> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>> Currently, the error handling for the call to function
>> ath10k_get_legacy_rate_idx() doesn't work because
>> *rate_idx* is of type u8 (8 bits, unsigned), which
>> makes it impossible for it to hold a value less
>> than 0.
>>
>> Fix this by changing the type of variable *rate_idx*
>> to s8 (8 bits, signed).
>
> There are more than 127 rates, are you sure this is doing
> what you want?
>
Based on the following function, rate_idx can only hold values from 0 to 11
static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
{
static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
18, 24, 36, 48, 54};
int i;
for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
if (rate == legacy_rates[i])
return i;
}
ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
return -EINVAL;
}
Thanks
--
Gustavo
> Thanks,
> Ben
>
>>
>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index f240525..edd0e74 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>> struct ath10k_per_peer_tx_stats *peer_stats)
>> {
>> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>> - u8 rate = 0, rate_idx = 0, sgi;
>> + u8 rate = 0, sgi;
>> + s8 rate_idx = 0;
>> struct rate_info txrate;
>>
>> lockdep_assert_held(&ar->data_lock);
>>
>
>
^ permalink raw reply
* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
From: Ben Greear @ 2018-10-05 19:09 UTC (permalink / raw)
To: Gustavo A. R. Silva, Kalle Valo, David S. Miller
Cc: ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <20181005184245.GA11700@embeddedor.com>
On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
> Currently, the error handling for the call to function
> ath10k_get_legacy_rate_idx() doesn't work because
> *rate_idx* is of type u8 (8 bits, unsigned), which
> makes it impossible for it to hold a value less
> than 0.
>
> Fix this by changing the type of variable *rate_idx*
> to s8 (8 bits, signed).
There are more than 127 rates, are you sure this is doing
what you want?
Thanks,
Ben
>
> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index f240525..edd0e74 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
> struct ath10k_per_peer_tx_stats *peer_stats)
> {
> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
> - u8 rate = 0, rate_idx = 0, sgi;
> + u8 rate = 0, sgi;
> + s8 rate_idx = 0;
> struct rate_info txrate;
>
> lockdep_assert_held(&ar->data_lock);
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox