* Re: [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Eric Dumazet @ 2018-09-27 22:18 UTC (permalink / raw)
To: Cong Wang, Eric Dumazet
Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim,
Vlad Buslov
In-Reply-To: <CAM_iQpX0sxzHT4aiaATPDHRnxWb-x1f_WhV9BQ58GNYZ=DUPHA@mail.gmail.com>
On 09/27/2018 02:36 PM, Cong Wang wrote:
> I don't understand what you mean by changing ip command, you must
> mean tc command, but still, I have no idea about how restarting failed
> syscall could be related to my patch and why we need to restart anything
> here. If the refcnt goes to 0, it will never come back, retrying won't help
> anything.
>
Yep, tc command it is.
I was not especially commenting your patch (replacing an english message by another does
not seem very big deal), but the fact that the code right there seems to be prepared
for parallel changes.
But using RCU lookups in control path will lead to occasional failures
that most user space tools would not expect.
Lets assume two tasks are launching "tc qdisc replace dev eth0 root XXX" in whatever order/parallelism.
Both should succeed, after/before major RTNL->other_locking_mechanism
Control paths are usually using a mutex or a spinlock so that they never hit a 0-refcount at all.
^ permalink raw reply
* Re: [PATCH v6] selftests: add headers_install to lib.mk
From: Michael Ellerman @ 2018-09-28 4:52 UTC (permalink / raw)
To: Anders Roxell, yamada.masahiro, michal.lkml, shuah, bamv2005,
brgl, pbonzini, akpm, rppt, aarcange
Cc: linux-kbuild, linux-kernel, linux-kselftest, netdev,
Anders Roxell, linuxppc-dev
In-Reply-To: <20180904104721.27535-1-anders.roxell@linaro.org>
[ + linuxppc-dev ]
Anders Roxell <anders.roxell@linaro.org> writes:
> If the kernel headers aren't installed we can't build all the tests.
> Add a new make target rule 'khdr' in the file lib.mk to generate the
> kernel headers and that gets include for every test-dir Makefile that
> includes lib.mk If the testdir in turn have its own sub-dirs the
> top_srcdir needs to be set to the linux-rootdir to be able to generate
> the kernel headers.
>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> Reviewed-by: Fathi Boudra <fathi.boudra@linaro.org>
> ---
>
> I sent this (v5) a month ago and wondered if it got lost. Resending
> unchanged.
>
> Cheers,
> Anders
>
> Makefile | 14 +-------------
> scripts/subarch.include | 13 +++++++++++++
> tools/testing/selftests/android/Makefile | 2 +-
> tools/testing/selftests/android/ion/Makefile | 2 ++
> tools/testing/selftests/futex/functional/Makefile | 1 +
> tools/testing/selftests/gpio/Makefile | 7 ++-----
> tools/testing/selftests/kvm/Makefile | 7 ++-----
> tools/testing/selftests/lib.mk | 12 ++++++++++++
> tools/testing/selftests/net/Makefile | 1 +
> .../selftests/networking/timestamping/Makefile | 1 +
> tools/testing/selftests/vm/Makefile | 4 ----
> 11 files changed, 36 insertions(+), 28 deletions(-)
> create mode 100644 scripts/subarch.include
This broke all the powerpc selftests :(
Why did it go in at rc5?
cheers
^ permalink raw reply
* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Eric Biggers @ 2018-09-28 4:55 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-kernel, netdev, linux-crypto, davem, gregkh
In-Reply-To: <20180928023546.GA20765@zx2c4.com>
Hi Jason,
On Fri, Sep 28, 2018 at 04:35:48AM +0200, Jason A. Donenfeld wrote:
> Hi Eric,
>
> On Thu, Sep 27, 2018 at 06:17:27PM -0700, Eric Biggers wrote:
> > So, Zinc will simultaneously replace the current crypto implementations, *and*
> > be "orthogonal" and "separate" from all the crypto code currently maintained by
> > Herbert? You can't have your cake and eat it too...
>
> The plan is for it to replace many uses of the crypto API where it makes
> sense, but not replace uses where it doesn't make sense. Perhaps in the
> long run, over time, its usage will grow to cover those cases too, or,
> perhaps instead, Zinc will form a simple basis of software crypto
> algorithms in whatever future API designs crop up. In other words, like
> most changes in kernel development, things happen gradually, starting
> with a few good cases, and gradually growing as the need and design
> arise.
Please re-read what I wrote. Here I'm talking about the crypto code itself, not
its users.
And you still haven't answered my question about adding a new algorithm that is
useful to have in both APIs. You're proposing that in most cases the crypto API
part will need to go through Herbert while the implementation will need to go
through you/Greg, right? Or will you/Greg be taking both? Or will Herbert take
both?
>
> > I'm still concerned you're splitting the community in two. It will be unclear
> > where new algorithms and implementations should go. Some people will choose
> > Herbert and the current crypto API and conventions, and some people will choose
> > you and Zinc... I still don't see clear guidelines for what will go where.
>
> I can try to work out some explicit guidelines and write these up for
> Documentation/, if that'd make a difference for you. I don't think this
> is a matter of "community splitting". On the contrary, I think Zinc will
> bring communities together, inviting the larger cryptography community
> to take an interest in improving the state of crypto in Linux. Either
> way, the litmus test for where code should go remains pretty similar to
> how it's been working so far. Are you tempted to stick it in lib/
> because that fits your programming paradigm and because you think it's
> generally useful? If so, submit it to lib/zinc/. Conversely, is it only
> something used in the large array of options provided by dmcrypt, ipsec,
> afalg, etc? Submit it to the crypto API.
>
> If you think this criteria is lacking, I'm amenable to adjusting that
> and changing it, especially as situations and designs change and morph
> over time. But that seems like a fairly decent starting point.
A documentation file for Zinc is a good idea. Some of the information in your
commit messages should be moved there too.
>
> > Please reach out to Herbert to find a sane solution
> > crypto development without choosing "sides".
>
> Please, don't politicize this. This has nothing to do with "sides". This
> has to do with which paradigm makes sense for implementing a particular
> algorithm.
I'm not trying to "politicize" this, but rather determine your criteria for
including algorithms in Zinc, which you haven't made clear. Since for the
second time you've avoided answering my question about whether you'd allow the
SM4 cipher in Zinc, and you designed WireGuard to be "cryptographically
opinionated", and you were one of the loudest voices calling for the Speck
cipher to be removed, and your justification for Zinc complains about cipher
modes from "90s cryptographers", I think it's reasonable for people to wonder
whether as the Zinc (i.e. Linux crypto library) maintainer you will restrict the
inclusion of crypto algorithms to the ones you prefer, rather than the ones that
users need. Note that the kernel is used by people all over the world and needs
to support various standards, protocols, and APIs that use different crypto
algorithms, not always the ones we'd like; and different users have different
preferences. People need to know whether the Linux kernel's crypto library will
meet (or be allowed to meet) their crypto needs.
> And everything that goes in Zinc gets to be used seamlessly
> by the crypto API anyway, through use of the trivial stub glue code,
> like what I've shown in the two commits in this series. Again, if it's
> something that will work well as a direct function call, then it seems
> like Zinc makes sense as a home for it.
>
> With that said, I've reached out to Herbert, and we'll of course discuss
> and reach a good conclusion together.
>
> > Note that usage can change over time; a user that requires a
> > single cipher could later need multiple, and vice versa.
>
> I think this depends on the design of the driver and the style it's
> implemented in. For example, I could imagine something like this:
>
> encrypt_stuff_with_morus(obj, key);
>
> evolving over time to:
>
> if (obj->type == MORUS_TYPE)
> encrypt_stuff_with_morus(obj, key);
> else if (obj->type == AEGIS_TYPE)
> encrypt_stuff_with_aegis(obj, key);
>
> On the other hand, if the developer has good reason to use the crypto
> API's dynamic dispatch and async API and so forth, then perhaps it just
> changes from:
>
> static const char *cipher_name = "morus";
>
> to
>
> static const char *cipher_name_type_1 = "morus";
> static const char *cipher_name_type_2 = "aegis";
>
> I can imagine both programming styles and evolutions being desirable for
> different reasons.
>
> > > - It matches exactly what Andy Polyakov's code is doing for the exact
> > > same reason, so this isn't something that's actually "new". (There
> > > are paths inside his implementation that branch from the vector code
> > > to the scalar code.)
> >
> > Matches Andy's code, where? The reason you had to add the radix conversion is
> > because his code does *not* handle it...
>
> For example, check out the avx blocks function. The radix conversion
> happens in a few different places throughout. The reason we need it
> separately here is because, unlike userspace, it's possible the kernel
> code will transition from 2^26 back to 2^64 as a result of the FPU
> context changing.
IOW, you had to rewrite the x86 assembly algorithm in C and make it use a
different Poly1305 context format. That's a major change, where bugs can be
introduced -- and at least one was introduced, in fact. I'd appreciate it if
you were more accurate in describing your modifications and the potential risks
involved.
And yes I know why converting radixes is needed, as I had pointed it out...
>
> As well, AndyP seems to like the idea of including this logic in the
> assembly instead of in C, if I understood our discussions correctly, so
> there's a decent chance this will migrate out of the glue code and into
> the assembly properly, which is probably a better place for it.
>
> > > - It has been discussed at length with Andy, including what kinds of
> > > proofs we'll need if we want to chop it down further (to remove that
> > > final reduction), and why we both don't want to do that yet, and so
> > > we go with the full carrying for the avoidance of risk.
> >
> > Sorry, other people don't know about your private discussions. For the rest of
> > us, why not add a comment to the code explaining what's going on?
>
> That's a good idea. I can include some discussion about this as well in
> the commit message that introduces the glue code, too, I guess? I've
> been hesitant to fill these commit messages up even more, given there
> are already so many walls of text and whatnot, but if you think that'd
> be useful, I'll do that for v7, and also add comments.
Please prefer to put information in the code or documentation rather than in
commit messages, when appropriate.
>
> > > - We've proved its correctness with Z3, actually using an even looser
> > > constraint on digit size than what Andy mentioned to us, thus resulting
> > > in a stronger proof result. So we're certain this isn't rubbish.
> > AFAICS actually it *is* rubbish, because your C code stores the accumulator as
> > 64-bit integers whereas the asm code (at least, the 32-bit version) reads it as
> > 32-bit integers. That won't work correctly on big endian ARM.
> > > There's no doubt about it, we've done our due-diligence here.
> > Apparently not, given that it's broken on big endian ARM.
> > Of course, having bugs in code which you insist was proven correct
> > + fuzzed doesn't exactly inspire trust.
>
> What's with the snark? It's not rubbish. I'm not sure if you noticed it in
> the development trees (both the WireGuard module tree and my kernel.org
> integration tree for this patch), but the big endian ARM support was fixed
> pretty shortly after I jumped the gun posting v6. Like, super soon after.
> That, and other big endian fixes (on aarch64 as well) are already queued up
> for v7. And now build.wireguard.com has more big endian running in CI.
>
> > The details of the correctness proofs and fuzzing you claim to have done aren't
> > explained, even in the cover letter; so for now we just have to trust you on
> > that point.
>
> "Claim to have done", "trust you on that point" -- I think there's no
> reason to doubt the integrity of my "claims", and I don't appreciate the
> phrasing that appears to call that into question.
>
> Regardless, sure, we can expand the "wall-of-text" commit messages even
> further, if you want, and include the verbatim Z3 scripts for reproduction.
>
> > I understand that your standards are still as high or even higher than
> > Herbert's, which is good; crypto code should be held to high standards! But
> > based on the evidence, I do worry there's a double standard going on where you
> > get away with things yourself which you won't allow from others in Zinc. It's
> > just not honest, and it will make people not want to contribute to Zinc.
> > Maintainers are supposed to be unbiased and hold all contributions to the same
> > standard.
>
> This is complete and utter garbage, and I find its insinuations insulting
> and ridiculous. There is absolutely no lack of honesty and no double
> standard being applied whatsoever. Your attempt to cast doubt about the
> quality of standards applied and the integrity of the process is wholly
> inappropriate. When I tell you that high standards were applied and that
> due-diligence was done in developing a particular patch, I mean what I
> say.
No, I was not aware of the fixed version. I found the bug myself reading the
latest patchset you've mailed out, v6. It's great that you found and fixed this
bug on your own, and of course that raises my level of confidence in your work.
Still, the v6 patchset still includes your claim that "All implementations have
been extensively tested and fuzzed"; so that claim was objectively wrong. So I
disagree that my concerns are "complete and utter garbage". And I think that
the fact that you prefer to respond by attacking me, rather than committing to
be more accurate in your claims and to treat all contributions fairly, is
problematic.
Also, if you have extra tests that you're running, it would be great if you
could include them as part of the submission so that others can run them too.
>
> > We need "Zinc" to be Linux's crypto library, not "Jason's crypto library".
>
> This very much is a project directed toward the benefit of the kernel in
> a general sense. It's been this way from the start, and there's nothing
> in its goals or plans to the contrary of that. Please leave this vague
> and unproductive rhetoric aside.
>
Well, it doesn't help that you yourself claim that Zinc stands for
"Zx2c4's INsane Cryptolib"...
- Eric
^ permalink raw reply
* [PATCH net-next 0/5] net: systemport: Turn on offloads by
From: Florian Fainelli @ 2018-09-27 22:36 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Hi David,
Up until now, we had added all the code necessary to turn on RX/TX
checksum offloads at runtime, but there is no reason why they have to be
disabled by default given that this gives a slight performance
improvement.
Florian Fainelli (5):
net: systemport: Refactor bcm_sysport_set_features()
net: systemport: Utilize bcm_sysport_set_features() during resume/open
net: systemport: Turn on offloads by default
net: systemport: Be drop monitor friendly while re-allocating headroom
net: systemport: Add software counters to track reallocations
drivers/net/ethernet/broadcom/bcmsysport.c | 67 +++++++++++-----------
drivers/net/ethernet/broadcom/bcmsysport.h | 2 +
2 files changed, 35 insertions(+), 34 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 1/5] net: systemport: Refactor bcm_sysport_set_features()
From: Florian Fainelli @ 2018-09-27 22:36 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <20180927223614.7116-1-f.fainelli@gmail.com>
In preparation for unconditionally enabling TX and RX checksum offloads,
refactor bcm_sysport_set_features() a bit such that
__netdev_update_features() during register_netdev() can make sure that
features are correctly programmed during network device registration.
Since we can now be called during register_netdev() with clocks gated,
we need to temporarily turn them on/off in order to have a successful
register programming.
We also move the CRC forward setting read into
bcm_sysport_set_features() since priv->crc_fwd matters while turning on
RX checksum offload, that way we are guaranteed they are in sync in case
we ever add support for NETIF_F_RXFCS at some point in the future.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 38 +++++++++-------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 147045757b10..654a07b849c4 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -126,8 +126,8 @@ static inline void tdma_port_write_desc_addr(struct bcm_sysport_priv *priv,
}
/* Ethtool operations */
-static int bcm_sysport_set_rx_csum(struct net_device *dev,
- netdev_features_t wanted)
+static void bcm_sysport_set_rx_csum(struct net_device *dev,
+ netdev_features_t wanted)
{
struct bcm_sysport_priv *priv = netdev_priv(dev);
u32 reg;
@@ -157,12 +157,10 @@ static int bcm_sysport_set_rx_csum(struct net_device *dev,
reg &= ~RXCHK_BRCM_TAG_EN;
rxchk_writel(priv, reg, RXCHK_CONTROL);
-
- return 0;
}
-static int bcm_sysport_set_tx_csum(struct net_device *dev,
- netdev_features_t wanted)
+static void bcm_sysport_set_tx_csum(struct net_device *dev,
+ netdev_features_t wanted)
{
struct bcm_sysport_priv *priv = netdev_priv(dev);
u32 reg;
@@ -177,23 +175,24 @@ static int bcm_sysport_set_tx_csum(struct net_device *dev,
else
reg &= ~tdma_control_bit(priv, TSB_EN);
tdma_writel(priv, reg, TDMA_CONTROL);
-
- return 0;
}
static int bcm_sysport_set_features(struct net_device *dev,
netdev_features_t features)
{
- netdev_features_t changed = features ^ dev->features;
- netdev_features_t wanted = dev->wanted_features;
- int ret = 0;
+ struct bcm_sysport_priv *priv = netdev_priv(dev);
+
+ /* Read CRC forward */
+ if (!priv->is_lite)
+ priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
+ else
+ priv->crc_fwd = !((gib_readl(priv, GIB_CONTROL) &
+ GIB_FCS_STRIP) >> GIB_FCS_STRIP_SHIFT);
- if (changed & NETIF_F_RXCSUM)
- ret = bcm_sysport_set_rx_csum(dev, wanted);
- if (changed & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))
- ret = bcm_sysport_set_tx_csum(dev, wanted);
+ bcm_sysport_set_rx_csum(dev, features);
+ bcm_sysport_set_tx_csum(dev, features);
- return ret;
+ return 0;
}
/* Hardware counters must be kept in sync because the order/offset
@@ -1976,13 +1975,6 @@ static int bcm_sysport_open(struct net_device *dev)
/* Set MAC address */
umac_set_hw_addr(priv, dev->dev_addr);
- /* Read CRC forward */
- if (!priv->is_lite)
- priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
- else
- priv->crc_fwd = !((gib_readl(priv, GIB_CONTROL) &
- GIB_FCS_STRIP) >> GIB_FCS_STRIP_SHIFT);
-
phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
0, priv->phy_interface);
if (!phydev) {
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/5] net: systemport: Utilize bcm_sysport_set_features() during resume/open
From: Florian Fainelli @ 2018-09-27 22:36 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <20180927223614.7116-1-f.fainelli@gmail.com>
During driver resume and open, the HW may have lost its context/state,
utilize bcm_sysport_set_features() to make sure we do restore the
correct set of features that were previously configured.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 654a07b849c4..3b4cb906a275 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1972,6 +1972,11 @@ static int bcm_sysport_open(struct net_device *dev)
else
gib_set_pad_extension(priv);
+ /* Apply features again in case we changed them while interface was
+ * down
+ */
+ bcm_sysport_set_features(dev, dev->features);
+
/* Set MAC address */
umac_set_hw_addr(priv, dev->dev_addr);
@@ -2708,7 +2713,6 @@ static int __maybe_unused bcm_sysport_resume(struct device *d)
struct net_device *dev = dev_get_drvdata(d);
struct bcm_sysport_priv *priv = netdev_priv(dev);
unsigned int i;
- u32 reg;
int ret;
if (!netif_running(dev))
@@ -2752,12 +2756,8 @@ static int __maybe_unused bcm_sysport_resume(struct device *d)
goto out_free_rx_ring;
}
- /* Enable rxhck */
- if (priv->rx_chk_en) {
- reg = rxchk_readl(priv, RXCHK_CONTROL);
- reg |= RXCHK_EN;
- rxchk_writel(priv, reg, RXCHK_CONTROL);
- }
+ /* Restore enabled features */
+ bcm_sysport_set_features(dev, dev->features);
rbuf_init(priv);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/5] net: systemport: Turn on offloads by default
From: Florian Fainelli @ 2018-09-27 22:36 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <20180927223614.7116-1-f.fainelli@gmail.com>
We can turn on the RX/TX checksum offloads by default and make sure that
those are properly reflected back to e.g: stacked devices such as VLAN
or DSA.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 3b4cb906a275..977d9dec2fb0 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -2508,9 +2508,10 @@ static int bcm_sysport_probe(struct platform_device *pdev)
dev->netdev_ops = &bcm_sysport_netdev_ops;
netif_napi_add(dev, &priv->napi, bcm_sysport_poll, 64);
- /* HW supported features, none enabled by default */
- dev->hw_features |= NETIF_F_RXCSUM | NETIF_F_HIGHDMA |
- NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+ dev->features |= NETIF_F_RXCSUM | NETIF_F_HIGHDMA |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+ dev->hw_features |= dev->features;
+ dev->vlan_features |= dev->features;
/* Request the WOL interrupt and advertise suspend if available */
priv->wol_irq_disabled = 1;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 4/5] net: systemport: Be drop monitor friendly while re-allocating headroom
From: Florian Fainelli @ 2018-09-27 22:36 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <20180927223614.7116-1-f.fainelli@gmail.com>
During bcm_sysport_insert_tsb() make sure we differentiate a SKB
headroom re-allocation failure from the normal swap and replace path.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 977d9dec2fb0..6c40cf6090ab 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1230,12 +1230,13 @@ static struct sk_buff *bcm_sysport_insert_tsb(struct sk_buff *skb,
/* Re-allocate SKB if needed */
if (unlikely(skb_headroom(skb) < sizeof(*tsb))) {
nskb = skb_realloc_headroom(skb, sizeof(*tsb));
- dev_kfree_skb(skb);
if (!nskb) {
+ dev_kfree_skb_any(skb);
dev->stats.tx_errors++;
dev->stats.tx_dropped++;
return NULL;
}
+ dev_consume_skb_any(skb);
skb = nskb;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 5/5] net: systemport: Add software counters to track reallocations
From: Florian Fainelli @ 2018-09-27 22:36 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <20180927223614.7116-1-f.fainelli@gmail.com>
When inserting the TSB, keep track of how many times we had to do it and
if there was a failure in doing so, this helps profile the driver for
possibly incorrect headroom settings.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 5 +++++
drivers/net/ethernet/broadcom/bcmsysport.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 6c40cf6090ab..faba55fd656a 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -284,6 +284,8 @@ static const struct bcm_sysport_stats bcm_sysport_gstrings_stats[] = {
STAT_MIB_SOFT("alloc_rx_buff_failed", mib.alloc_rx_buff_failed),
STAT_MIB_SOFT("rx_dma_failed", mib.rx_dma_failed),
STAT_MIB_SOFT("tx_dma_failed", mib.tx_dma_failed),
+ STAT_MIB_SOFT("tx_realloc_tsb", mib.tx_realloc_tsb),
+ STAT_MIB_SOFT("tx_realloc_tsb_failed", mib.tx_realloc_tsb_failed),
/* Per TX-queue statistics are dynamically appended */
};
@@ -1220,6 +1222,7 @@ static void bcm_sysport_poll_controller(struct net_device *dev)
static struct sk_buff *bcm_sysport_insert_tsb(struct sk_buff *skb,
struct net_device *dev)
{
+ struct bcm_sysport_priv *priv = netdev_priv(dev);
struct sk_buff *nskb;
struct bcm_tsb *tsb;
u32 csum_info;
@@ -1232,12 +1235,14 @@ static struct sk_buff *bcm_sysport_insert_tsb(struct sk_buff *skb,
nskb = skb_realloc_headroom(skb, sizeof(*tsb));
if (!nskb) {
dev_kfree_skb_any(skb);
+ priv->mib.tx_realloc_tsb_failed++;
dev->stats.tx_errors++;
dev->stats.tx_dropped++;
return NULL;
}
dev_consume_skb_any(skb);
skb = nskb;
+ priv->mib.tx_realloc_tsb++;
}
tsb = skb_push(skb, sizeof(*tsb));
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index 046c6c1d97fd..a7a230884a87 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -607,6 +607,8 @@ struct bcm_sysport_mib {
u32 alloc_rx_buff_failed;
u32 rx_dma_failed;
u32 tx_dma_failed;
+ u32 tx_realloc_tsb;
+ u32 tx_realloc_tsb_failed;
};
/* HW maintains a large list of counters */
--
2.17.1
^ permalink raw reply related
* Dear Friend i need your help
From: Aisha Gaddafi @ 2018-09-28 0:38 UTC (permalink / raw)
--
Dear Assalamu Alaikum,
I came across your contact during my private search
Mrs Aisha Al-Qaddafi is my name, the only daughter of late Libyan
president, I have funds the sum
of $27.5 million USD for investment, I am interested in you for
investment project assistance in your country,
i shall compensate you 30% of the total sum after the funds are
transfer into your account,
Greetings from Mrs Aisha Al-Qaddafi
Mrs Aisha Al-Qaddafi
^ permalink raw reply
* [PATCH 01/11] net: ip_rt_get_source() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
From: Maciej Żenczykowski <maze@google.com>
(allows for better compiler optimization)
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv4/route.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index dce2ed66ebe1..02482b71498b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1217,18 +1217,15 @@ void ip_rt_get_source(u8 *addr, struct sk_buff *skb, struct rtable *rt)
src = ip_hdr(skb)->saddr;
else {
struct fib_result res;
- struct flowi4 fl4;
- struct iphdr *iph;
-
- iph = ip_hdr(skb);
-
- memset(&fl4, 0, sizeof(fl4));
- fl4.daddr = iph->daddr;
- fl4.saddr = iph->saddr;
- fl4.flowi4_tos = RT_TOS(iph->tos);
- fl4.flowi4_oif = rt->dst.dev->ifindex;
- fl4.flowi4_iif = skb->dev->ifindex;
- fl4.flowi4_mark = skb->mark;
+ struct iphdr *iph = ip_hdr(skb);
+ struct flowi4 fl4 = {
+ .daddr = iph->daddr,
+ .saddr = iph->saddr,
+ .flowi4_tos = RT_TOS(iph->tos),
+ .flowi4_oif = rt->dst.dev->ifindex,
+ .flowi4_iif = skb->dev->ifindex,
+ .flowi4_mark = skb->mark,
+ };
rcu_read_lock();
if (fib_lookup(dev_net(rt->dst.dev), &fl4, &res, 0) == 0)
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 02/11] net: inet_rtm_getroute() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv4/route.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 02482b71498b..048919713f4e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2780,7 +2780,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
struct rtable *rt = NULL;
struct sk_buff *skb;
struct rtmsg *rtm;
- struct flowi4 fl4;
+ struct flowi4 fl4 = {};
__be32 dst = 0;
__be32 src = 0;
kuid_t uid;
@@ -2820,7 +2820,6 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
if (!skb)
return -ENOBUFS;
- memset(&fl4, 0, sizeof(fl4));
fl4.daddr = dst;
fl4.saddr = src;
fl4.flowi4_tos = rtm->rtm_tos;
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 03/11] net: fib_multipath_hash() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv4/route.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 048919713f4e..17953a52fbd0 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1821,12 +1821,11 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
const struct sk_buff *skb, struct flow_keys *flkeys)
{
- struct flow_keys hash_keys;
+ struct flow_keys hash_keys = {};
u32 mhash;
switch (net->ipv4.sysctl_fib_multipath_hash_policy) {
case 0:
- memset(&hash_keys, 0, sizeof(hash_keys));
hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
if (skb) {
ip_multipath_l3_keys(skb, &hash_keys);
@@ -1845,8 +1844,6 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
if (skb->l4_hash)
return skb_get_hash_raw(skb) >> 1;
- memset(&hash_keys, 0, sizeof(hash_keys));
-
if (!flkeys) {
skb_flow_dissect_flow_keys(skb, &keys, flag);
flkeys = &keys;
@@ -1859,7 +1856,6 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
hash_keys.ports.dst = flkeys->ports.dst;
hash_keys.basic.ip_proto = flkeys->basic.ip_proto;
} else {
- memset(&hash_keys, 0, sizeof(hash_keys));
hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
hash_keys.addrs.v4addrs.src = fl4->saddr;
hash_keys.addrs.v4addrs.dst = fl4->daddr;
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 04/11] net: ip6_multipath_l3_keys() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d28f83e01593..9cb024451fc5 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1981,12 +1981,11 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
const struct sk_buff *skb, struct flow_keys *flkeys)
{
- struct flow_keys hash_keys;
+ struct flow_keys hash_keys = {};
u32 mhash;
switch (ip6_multipath_hash_policy(net)) {
case 0:
- memset(&hash_keys, 0, sizeof(hash_keys));
hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
if (skb) {
ip6_multipath_l3_keys(skb, &hash_keys, flkeys);
@@ -2006,8 +2005,6 @@ u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
if (skb->l4_hash)
return skb_get_hash_raw(skb) >> 1;
- memset(&hash_keys, 0, sizeof(hash_keys));
-
if (!flkeys) {
skb_flow_dissect_flow_keys(skb, &keys, flag);
flkeys = &keys;
@@ -2019,7 +2016,6 @@ u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
hash_keys.ports.dst = flkeys->ports.dst;
hash_keys.basic.ip_proto = flkeys->basic.ip_proto;
} else {
- memset(&hash_keys, 0, sizeof(hash_keys));
hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
hash_keys.addrs.v6addrs.src = fl6->saddr;
hash_keys.addrs.v6addrs.dst = fl6->daddr;
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 05/11] net: ip6_redirect() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
(allows for better compiler optimization)
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9cb024451fc5..e148d197d628 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2504,16 +2504,15 @@ void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark,
{
const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data;
struct dst_entry *dst;
- struct flowi6 fl6;
-
- memset(&fl6, 0, sizeof(fl6));
- fl6.flowi6_iif = LOOPBACK_IFINDEX;
- fl6.flowi6_oif = oif;
- fl6.flowi6_mark = mark;
- fl6.daddr = iph->daddr;
- fl6.saddr = iph->saddr;
- fl6.flowlabel = ip6_flowinfo(iph);
- fl6.flowi6_uid = uid;
+ struct flowi6 fl6 = {
+ .flowi6_iif = LOOPBACK_IFINDEX,
+ .flowi6_oif = oif,
+ .flowi6_mark = mark,
+ .daddr = iph->daddr,
+ .saddr = iph->saddr,
+ .flowlabel = ip6_flowinfo(iph),
+ .flowi6_uid = uid,
+ };
dst = ip6_route_redirect(net, &fl6, skb, &ipv6_hdr(skb)->saddr);
rt6_do_redirect(dst, NULL, skb);
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 06/11] net: ip6_redirect_no_header() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
(allows for better compiler optimization)
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index e148d197d628..dd8c04f253d5 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2526,15 +2526,14 @@ void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
const struct ipv6hdr *iph = ipv6_hdr(skb);
const struct rd_msg *msg = (struct rd_msg *)icmp6_hdr(skb);
struct dst_entry *dst;
- struct flowi6 fl6;
-
- memset(&fl6, 0, sizeof(fl6));
- fl6.flowi6_iif = LOOPBACK_IFINDEX;
- fl6.flowi6_oif = oif;
- fl6.flowi6_mark = mark;
- fl6.daddr = msg->dest;
- fl6.saddr = iph->daddr;
- fl6.flowi6_uid = sock_net_uid(net, NULL);
+ struct flowi6 fl6 = {
+ .flowi6_iif = LOOPBACK_IFINDEX,
+ .flowi6_oif = oif,
+ .flowi6_mark = mark,
+ .daddr = msg->dest,
+ .saddr = iph->daddr,
+ .flowi6_uid = sock_net_uid(net, NULL),
+ };
dst = ip6_route_redirect(net, &fl6, skb, &iph->saddr);
rt6_do_redirect(dst, NULL, skb);
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 07/11] net: remove 1 always zero parameter from ip6_redirect_no_header()
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
(the parameter in question is mark)
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
include/net/ip6_route.h | 3 +--
net/ipv6/ndisc.c | 2 +-
net/ipv6/route.c | 4 +---
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 7b9c82de11cc..cef186dbd2ce 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -165,8 +165,7 @@ void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu, int oif,
void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, __be32 mtu);
void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark,
kuid_t uid);
-void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
- u32 mark);
+void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif);
void ip6_sk_redirect(struct sk_buff *skb, struct sock *sk);
struct netlink_callback;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 0ec273997d1d..51863ada15a4 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1533,7 +1533,7 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
if (!ndopts.nd_opts_rh) {
ip6_redirect_no_header(skb, dev_net(skb->dev),
- skb->dev->ifindex, 0);
+ skb->dev->ifindex);
return;
}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index dd8c04f253d5..27f1260e053a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2520,8 +2520,7 @@ void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark,
}
EXPORT_SYMBOL_GPL(ip6_redirect);
-void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
- u32 mark)
+void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif)
{
const struct ipv6hdr *iph = ipv6_hdr(skb);
const struct rd_msg *msg = (struct rd_msg *)icmp6_hdr(skb);
@@ -2529,7 +2528,6 @@ void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
struct flowi6 fl6 = {
.flowi6_iif = LOOPBACK_IFINDEX,
.flowi6_oif = oif,
- .flowi6_mark = mark,
.daddr = msg->dest,
.saddr = iph->daddr,
.flowi6_uid = sock_net_uid(net, NULL),
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 08/11] net: ip6_update_pmtu() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
(allows for better compiler optimization)
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 27f1260e053a..a87b79574a91 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2345,15 +2345,14 @@ void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu,
{
const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data;
struct dst_entry *dst;
- struct flowi6 fl6;
-
- memset(&fl6, 0, sizeof(fl6));
- fl6.flowi6_oif = oif;
- fl6.flowi6_mark = mark ? mark : IP6_REPLY_MARK(net, skb->mark);
- fl6.daddr = iph->daddr;
- fl6.saddr = iph->saddr;
- fl6.flowlabel = ip6_flowinfo(iph);
- fl6.flowi6_uid = uid;
+ struct flowi6 fl6 = {
+ .flowi6_oif = oif,
+ .flowi6_mark = mark ? mark : IP6_REPLY_MARK(net, skb->mark),
+ .daddr = iph->daddr,
+ .saddr = iph->saddr,
+ .flowlabel = ip6_flowinfo(iph),
+ .flowi6_uid = uid,
+ };
dst = ip6_route_output(net, NULL, &fl6);
if (!dst->error)
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 09/11] net: rtmsg_to_fib6_config() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
(allows for better compiler optimization)
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index a87b79574a91..b8fece1d6021 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3600,23 +3600,23 @@ static void rtmsg_to_fib6_config(struct net *net,
struct in6_rtmsg *rtmsg,
struct fib6_config *cfg)
{
- memset(cfg, 0, sizeof(*cfg));
+ *cfg = (struct fib6_config){
+ .fc_table = l3mdev_fib_table_by_index(net, rtmsg->rtmsg_ifindex) ?
+ : RT6_TABLE_MAIN,
+ .fc_ifindex = rtmsg->rtmsg_ifindex,
+ .fc_metric = rtmsg->rtmsg_metric,
+ .fc_expires = rtmsg->rtmsg_info,
+ .fc_dst_len = rtmsg->rtmsg_dst_len,
+ .fc_src_len = rtmsg->rtmsg_src_len,
+ .fc_flags = rtmsg->rtmsg_flags,
+ .fc_type = rtmsg->rtmsg_type,
+
+ .fc_nlinfo.nl_net = net,
- cfg->fc_table = l3mdev_fib_table_by_index(net, rtmsg->rtmsg_ifindex) ?
- : RT6_TABLE_MAIN;
- cfg->fc_ifindex = rtmsg->rtmsg_ifindex;
- cfg->fc_metric = rtmsg->rtmsg_metric;
- cfg->fc_expires = rtmsg->rtmsg_info;
- cfg->fc_dst_len = rtmsg->rtmsg_dst_len;
- cfg->fc_src_len = rtmsg->rtmsg_src_len;
- cfg->fc_flags = rtmsg->rtmsg_flags;
- cfg->fc_type = rtmsg->rtmsg_type;
-
- cfg->fc_nlinfo.nl_net = net;
-
- cfg->fc_dst = rtmsg->rtmsg_dst;
- cfg->fc_src = rtmsg->rtmsg_src;
- cfg->fc_gateway = rtmsg->rtmsg_gateway;
+ .fc_dst = rtmsg->rtmsg_dst,
+ .fc_src = rtmsg->rtmsg_src,
+ .fc_gateway = rtmsg->rtmsg_gateway,
+ };
}
int ipv6_route_ioctl(struct net *net, unsigned int cmd, void __user *arg)
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 10/11] net: rtm_to_fib6_config() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
(allows for better compiler optimization)
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b8fece1d6021..9aca81772c93 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4139,14 +4139,19 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
err = -EINVAL;
rtm = nlmsg_data(nlh);
- memset(cfg, 0, sizeof(*cfg));
- cfg->fc_table = rtm->rtm_table;
- cfg->fc_dst_len = rtm->rtm_dst_len;
- cfg->fc_src_len = rtm->rtm_src_len;
- cfg->fc_flags = RTF_UP;
- cfg->fc_protocol = rtm->rtm_protocol;
- cfg->fc_type = rtm->rtm_type;
+ *cfg = (struct fib6_config){
+ .fc_table = rtm->rtm_table,
+ .fc_dst_len = rtm->rtm_dst_len,
+ .fc_src_len = rtm->rtm_src_len,
+ .fc_flags = RTF_UP,
+ .fc_protocol = rtm->rtm_protocol,
+ .fc_type = rtm->rtm_type,
+
+ .fc_nlinfo.portid = NETLINK_CB(skb).portid,
+ .fc_nlinfo.nlh = nlh,
+ .fc_nlinfo.nl_net = sock_net(skb->sk),
+ };
if (rtm->rtm_type == RTN_UNREACHABLE ||
rtm->rtm_type == RTN_BLACKHOLE ||
@@ -4162,10 +4167,6 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
cfg->fc_flags |= (rtm->rtm_flags & RTNH_F_ONLINK);
- cfg->fc_nlinfo.portid = NETLINK_CB(skb).portid;
- cfg->fc_nlinfo.nlh = nlh;
- cfg->fc_nlinfo.nl_net = sock_net(skb->sk);
-
if (tb[RTA_GATEWAY]) {
cfg->fc_gateway = nla_get_in6_addr(tb[RTA_GATEWAY]);
cfg->fc_flags |= RTF_GATEWAY;
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH 11/11] net: inet6_rtm_getroute() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-09-27 23:00 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-1-zenczykowski@gmail.com>
From: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9aca81772c93..aca6a84de794 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4819,7 +4819,7 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
struct rt6_info *rt;
struct sk_buff *skb;
struct rtmsg *rtm;
- struct flowi6 fl6;
+ struct flowi6 fl6 = {};
bool fibmatch;
err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv6_policy,
@@ -4828,7 +4828,6 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
goto errout;
err = -EINVAL;
- memset(&fl6, 0, sizeof(fl6));
rtm = nlmsg_data(nlh);
fl6.flowlabel = ip6_make_flowinfo(rtm->rtm_tos, 0);
fibmatch = !!(rtm->rtm_flags & RTM_F_FIB_MATCH);
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH] cfg80211: fix use-after-free in reg_process_hint()
From: Yu Zhao @ 2018-09-27 23:05 UTC (permalink / raw)
To: Johannes Berg
Cc: David S. Miller, linux-wireless, netdev, linux-kernel, Yu Zhao
reg_process_hint_country_ie() can free regulatory_request and return
REG_REQ_ALREADY_SET. We shouldn't use regulatory_request after it's
called. KASAN error was observed when this happens.
BUG: KASAN: use-after-free in reg_process_hint+0x839/0x8aa [cfg80211]
Read of size 4 at addr ffff8800c430d434 by task kworker/1:3/89
<snipped>
Workqueue: events reg_todo [cfg80211]
Call Trace:
dump_stack+0xc1/0x10c
? _atomic_dec_and_lock+0x1ad/0x1ad
? _raw_spin_lock_irqsave+0xa0/0xd2
print_address_description+0x86/0x26f
? reg_process_hint+0x839/0x8aa [cfg80211]
kasan_report+0x241/0x29b
reg_process_hint+0x839/0x8aa [cfg80211]
reg_todo+0x204/0x5b9 [cfg80211]
process_one_work+0x55f/0x8d0
? worker_detach_from_pool+0x1b5/0x1b5
? _raw_spin_unlock_irq+0x65/0xdd
? _raw_spin_unlock_irqrestore+0xf3/0xf3
worker_thread+0x5dd/0x841
? kthread_parkme+0x1d/0x1d
kthread+0x270/0x285
? pr_cont_work+0xe3/0xe3
? rcu_read_unlock_sched_notrace+0xca/0xca
ret_from_fork+0x22/0x40
Allocated by task 2718:
set_track+0x63/0xfa
__kmalloc+0x119/0x1ac
regulatory_hint_country_ie+0x38/0x329 [cfg80211]
__cfg80211_connect_result+0x854/0xadd [cfg80211]
cfg80211_rx_assoc_resp+0x3bc/0x4f0 [cfg80211]
smsc95xx v1.0.6
ieee80211_sta_rx_queued_mgmt+0x1803/0x7ed5 [mac80211]
ieee80211_iface_work+0x411/0x696 [mac80211]
process_one_work+0x55f/0x8d0
worker_thread+0x5dd/0x841
kthread+0x270/0x285
ret_from_fork+0x22/0x40
Freed by task 89:
set_track+0x63/0xfa
kasan_slab_free+0x6a/0x87
kfree+0xdc/0x470
reg_process_hint+0x31e/0x8aa [cfg80211]
reg_todo+0x204/0x5b9 [cfg80211]
process_one_work+0x55f/0x8d0
worker_thread+0x5dd/0x841
kthread+0x270/0x285
ret_from_fork+0x22/0x40
<snipped>
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
net/wireless/reg.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 765dedb12361..24cfa2776f50 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2661,11 +2661,12 @@ static void reg_process_hint(struct regulatory_request *reg_request)
{
struct wiphy *wiphy = NULL;
enum reg_request_treatment treatment;
+ enum nl80211_reg_initiator initiator = reg_request->initiator;
if (reg_request->wiphy_idx != WIPHY_IDX_INVALID)
wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
- switch (reg_request->initiator) {
+ switch (initiator) {
case NL80211_REGDOM_SET_BY_CORE:
treatment = reg_process_hint_core(reg_request);
break;
@@ -2683,7 +2684,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
treatment = reg_process_hint_country_ie(wiphy, reg_request);
break;
default:
- WARN(1, "invalid initiator %d\n", reg_request->initiator);
+ WARN(1, "invalid initiator %d\n", initiator);
goto out_free;
}
@@ -2698,7 +2699,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
*/
if (treatment == REG_REQ_ALREADY_SET && wiphy &&
wiphy->regulatory_flags & REGULATORY_STRICT_REG) {
- wiphy_update_regulatory(wiphy, reg_request->initiator);
+ wiphy_update_regulatory(wiphy, initiator);
wiphy_all_share_dfs_chan_state(wiphy);
reg_check_channels();
}
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: KMSAN: uninit-value in __dev_mc_add
From: Vladis Dronov @ 2018-09-27 23:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: syzbot+001516d86dbe88862cec, David Miller, netdev, LKML,
syzkaller-bugs
In-Reply-To: <CANn89iJNNfiEPM24NgnodV-QUnCyJcrRjKsHV9jdP4i3fBdESw@mail.gmail.com>
Hello, Eric, all,
> I dunno, your patch looks quite not the right fix.
I agree, it looks more like a dirty hack. Unfortunately, I lack the deep
expertise in the network stack subsystem, so I've posted the patch to,
sort of, start a discussion and probably get some hints.
> If TUN is able to change dev->type, how comes it does not set the
> appropriate dev->addr_len at the same time ?
Well,... probably, nobody cared to do so:
[drivers/net/tun.c]
case TUNSETLINK:
...
tun->dev->type = (int) arg; //<--- that's all!
tun_debug(KERN_INFO, tun, "linktype set to %d\n",
tun->dev->type);
ret = 0;
}
break;
> Really the bug seems to be deeper, and without setting proper
> dev->addr_len, we'll need more 'fixes' like yours.
Absolutely. Unfortunately, I wasn't able to just write such deeper patch.
Let me share what I have found and let me hope to get an advise.
- So setting just the tun->dev->type makes the dev struct inconsistent.
- There are more field to adjust, at least dev->broadcast. Also, there are
a number of *_ops fields which are all set for the Ethernet type, most
probably they must be adjusted also.
- There is no get_addr_len_by_link_type() or a simple way to get link layer
properties by dev->type. Such settings are scattered in *_setup and
*_init functions, like ipgre_tunnel_init() { ... dev->addr_len = 4; ...}
Having these, I can imagine 2 ways for a proper fix.
1) Destroy the net_device in question and recreate it when changing a link
type. This way all the dev fields are set right. Create it in a similar way
as rtnl_newlink() does. Again, we do not have get_X_by_link_type(), so it
probably will be some large switch()/case:
$ grep '^#define ARPHRD_' include/uapi/linux/if_arp.h | wc -l
59
2) Leave tun an Ethernet device, add some tun->pretend_to_be_this_link_type
field and change only it on TUNSETLINK. And use this field in cases for which
TUNSETLINK was invented in the first place. Unfortunately, I do not have such
a list. The initial the commit ff4cc3ac93e1 says:
For use with
wireless and other networking types it should be possible to set the
ARP type via an ioctl.
Surely, there can be something else which I do not see. Could anyone suggest
an advice on this?
Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security Engineer
^ permalink raw reply
* [PATCHv3 bpf-next 00/12] Add socket lookup support
From: Joe Stringer @ 2018-09-27 23:26 UTC (permalink / raw)
To: daniel
Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
This series proposes a new helper for the BPF API which allows BPF programs to
perform lookups for sockets in a network namespace. This would allow programs
to determine early on in processing whether the stack is expecting to receive
the packet, and perform some action (eg drop, forward somewhere) based on this
information.
The series is structured roughly into:
* Misc refactor
* Add the socket pointer type
* Add reference tracking to ensure that socket references are freed
* Extend the BPF API to add sk_lookup_xxx() / sk_release() functions
* Add tests/documentation
The helper proposed in this series includes a parameter for a tuple which must
be filled in by the caller to determine the socket to look up. The simplest
case would be filling with the contents of the packet, ie mapping the packet's
5-tuple into the parameter. In common cases, it may alternatively be useful to
reverse the direction of the tuple and perform a lookup, to find the socket
that initiates this connection; and if the BPF program ever performs a form of
IP address translation, it may further be useful to be able to look up
arbitrary tuples that are not based upon the packet, but instead based on state
held in BPF maps or hardcoded in the BPF program.
Currently, access into the socket's fields are limited to those which are
otherwise already accessible, and are restricted to read-only access.
Changes since v2:
* New patch: "selftests/bpf: Generalize dummy program types".
This enables adding verifier tests for socket lookup with tail calls.
* Define the semantics of the new helpers more clearly in uAPI header.
* Fix release of caller_net when netns is not specified.
* Use skb->sk to find caller net when skb->dev is unavailable.
* Fix build with !CONFIG_NET.
* Replace ptr_id defensive coding when releasing reference state with an
internal error (-EFAULT).
* Remove flags argument to sk_release().
* Add several new assembly tests suggested by Daniel.
* Add a few new C tests.
* Fix typo in verifier error message.
Changes since v1:
* Limit netns_id field to 32 bits
* Reuse reg_type_mismatch() in more places
* Reduce the number of passes at convert_ctx_access()
* Replace ptr_id defensive coding when releasing reference state with an
internal error (-EFAULT)
* Rework 'struct bpf_sock_tuple' to allow passing a packet pointer
* Allow direct packet access from helper
* Fix compile error with CONFIG_IPV6 enabled
* Improve commit messages
Changes since RFC:
* Split up sk_lookup() into sk_lookup_tcp(), sk_lookup_udp().
* Only take references on the socket when necessary.
* Make sk_release() only free the socket reference in this case.
* Fix some runtime reference leaks:
* Disallow BPF_LD_[ABS|IND] instructions while holding a reference.
* Disallow bpf_tail_call() while holding a reference.
* Prevent the same instruction being used for reference and other
pointer type.
* Simplify locating copies of a reference during helper calls by caching
the pointer id from the caller.
* Fix kbuild compilation warnings with particular configs.
* Improve code comments describing the new verifier pieces.
* Testing courtesy of Nitin
This tree is also available at:
https://github.com/joestringer/linux/commits/submit/sk-lookup-v3
Joe Stringer (12):
bpf: Add iterator for spilled registers
bpf: Simplify ptr_min_max_vals adjustment
bpf: Generalize ptr_or_null regs check
bpf: Add PTR_TO_SOCKET verifier type
bpf: Macrofy stack state copy
bpf: Add reference tracking to verifier
bpf: Add helper to retrieve socket in BPF
selftests/bpf: Generalize dummy program types
selftests/bpf: Add tests for reference tracking
libbpf: Support loading individual progs
selftests/bpf: Add C tests for reference tracking
Documentation: Describe bpf reference tracking
Documentation/networking/filter.txt | 64 ++
include/linux/bpf.h | 34 +
include/linux/bpf_verifier.h | 37 +-
include/uapi/linux/bpf.h | 93 ++-
kernel/bpf/verifier.c | 594 +++++++++++++---
net/core/filter.c | 181 ++++-
tools/include/uapi/linux/bpf.h | 93 ++-
tools/lib/bpf/libbpf.c | 4 +-
tools/lib/bpf/libbpf.h | 3 +
tools/testing/selftests/bpf/Makefile | 2 +-
tools/testing/selftests/bpf/bpf_helpers.h | 12 +
tools/testing/selftests/bpf/test_progs.c | 38 +
.../selftests/bpf/test_sk_lookup_kern.c | 180 +++++
tools/testing/selftests/bpf/test_verifier.c | 670 +++++++++++++++++-
14 files changed, 1858 insertions(+), 147 deletions(-)
create mode 100644 tools/testing/selftests/bpf/test_sk_lookup_kern.c
--
2.17.1
^ permalink raw reply
* [PATCHv3 bpf-next 01/12] bpf: Add iterator for spilled registers
From: Joe Stringer @ 2018-09-27 23:26 UTC (permalink / raw)
To: daniel
Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180927232659.14348-1-joe@wand.net.nz>
Add this iterator for spilled registers, it concentrates the details of
how to get the current frame's spilled registers into a single macro
while clarifying the intention of the code which is calling the macro.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf_verifier.h | 11 +++++++++++
kernel/bpf/verifier.c | 16 +++++++---------
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index b42b60a83e19..af262b97f586 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -131,6 +131,17 @@ struct bpf_verifier_state {
u32 curframe;
};
+#define __get_spilled_reg(slot, frame) \
+ (((slot < frame->allocated_stack / BPF_REG_SIZE) && \
+ (frame->stack[slot].slot_type[0] == STACK_SPILL)) \
+ ? &frame->stack[slot].spilled_ptr : NULL)
+
+/* Iterate over 'frame', setting 'reg' to either NULL or a spilled register. */
+#define for_each_spilled_reg(iter, frame, reg) \
+ for (iter = 0, reg = __get_spilled_reg(iter, frame); \
+ iter < frame->allocated_stack / BPF_REG_SIZE; \
+ iter++, reg = __get_spilled_reg(iter, frame))
+
/* linked list of verifier states used to prune search */
struct bpf_verifier_state_list {
struct bpf_verifier_state state;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a8cc83a970d1..18347de310ad 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2252,10 +2252,9 @@ static void __clear_all_pkt_pointers(struct bpf_verifier_env *env,
if (reg_is_pkt_pointer_any(®s[i]))
mark_reg_unknown(env, regs, i);
- for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
- if (state->stack[i].slot_type[0] != STACK_SPILL)
+ for_each_spilled_reg(i, state, reg) {
+ if (!reg)
continue;
- reg = &state->stack[i].spilled_ptr;
if (reg_is_pkt_pointer_any(reg))
__mark_reg_unknown(reg);
}
@@ -3395,10 +3394,9 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *vstate,
for (j = 0; j <= vstate->curframe; j++) {
state = vstate->frame[j];
- for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
- if (state->stack[i].slot_type[0] != STACK_SPILL)
+ for_each_spilled_reg(i, state, reg) {
+ if (!reg)
continue;
- reg = &state->stack[i].spilled_ptr;
if (reg->type == type && reg->id == dst_reg->id)
reg->range = max(reg->range, new_range);
}
@@ -3643,7 +3641,7 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
bool is_null)
{
struct bpf_func_state *state = vstate->frame[vstate->curframe];
- struct bpf_reg_state *regs = state->regs;
+ struct bpf_reg_state *reg, *regs = state->regs;
u32 id = regs[regno].id;
int i, j;
@@ -3652,8 +3650,8 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
for (j = 0; j <= vstate->curframe; j++) {
state = vstate->frame[j];
- for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
- if (state->stack[i].slot_type[0] != STACK_SPILL)
+ for_each_spilled_reg(i, state, reg) {
+ if (!reg)
continue;
mark_map_reg(&state->stack[i].spilled_ptr, 0, id, is_null);
}
--
2.17.1
^ permalink raw reply related
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