* [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 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 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 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 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
* 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
* 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] 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
* [PATCH bpf-next] bpf: permit CGROUP_DEVICE programs accessing helper bpf_get_current_cgroup_id()
From: Yonghong Song @ 2018-09-27 21:37 UTC (permalink / raw)
To: ast, daniel, netdev, guro; +Cc: kernel-team
Currently, helper bpf_get_current_cgroup_id() is not permitted
for CGROUP_DEVICE type of programs. If the helper is used
in such cases, the verifier will log the following error:
0: (bf) r6 = r1
1: (69) r7 = *(u16 *)(r6 +0)
2: (85) call bpf_get_current_cgroup_id#80
unknown func bpf_get_current_cgroup_id#80
The bpf_get_current_cgroup_id() is useful for CGROUP_DEVICE
type of programs in order to customize action based on cgroup id.
This patch added such a support.
Cc: Roman Gushchin <guro@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/cgroup.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 065c3d9ff8eb..00f6ed2e4f9a 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -707,6 +707,8 @@ cgroup_dev_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_get_current_uid_gid_proto;
case BPF_FUNC_get_local_storage:
return &bpf_get_local_storage_proto;
+ case BPF_FUNC_get_current_cgroup_id:
+ return &bpf_get_current_cgroup_id_proto;
case BPF_FUNC_trace_printk:
if (capable(CAP_SYS_ADMIN))
return bpf_get_trace_printk_proto();
--
2.17.1
^ permalink raw reply related
* Re: [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Cong Wang @ 2018-09-27 21:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim,
Vlad Buslov
In-Reply-To: <ec21bec3-2aa8-790d-6093-ba1522274615@gmail.com>
On Thu, Sep 27, 2018 at 2:16 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/27/2018 01:42 PM, Cong Wang wrote:
> > It is clearly a copy-n-paste.
> >
> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> > ---
> > net/sched/cls_api.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index 3de47e99b788..8dd7f8af6d54 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
> > @@ -655,7 +655,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
> >
> > *q = qdisc_refcount_inc_nz(*q);
> > if (!*q) {
> > - NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
> > + NL_SET_ERR_MSG(extack, "Can't increase Qdisc refcount");
>
>
> I am not sure it was a copy-n-paste.
Make sure you knew there is an exactly same extack message
(with a same English grammar error).
>
> Qdisc refcount business is kernel internal.
Yeah, but the extack message is already there, this patch doesn't add
any new extack. Or you are suggesting we should remove it?
> If we can not increase the refcount, this is precisely because this qdisc is about
> to be destroyed. Nothing fundamentally different than having this thread delayed a bit
> and qdisc_lookup_rcu() returning NULL in the first place.
qdisc_lookup_rcu() is not always called, it could be dev->qdisc.
I am pretty sure parent exists in dev->qdisc.
>
> This also means that using RCU for control path is problematic, as surely the caller
> of this interface would prefer something that succeeds, even if this means
> waiting a bit in the kernel.
I fail to validate this statement, Why it prefers success when refcnt reaches
0?
>
> Or are we willing to change ip command and make it restart failed syscalls ?
>
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.
BTW:
If you have any other question beyond my patch's scope, isn't it better
that we start a new thread for discussion?
In case you still misunderstand, my patch never intends to address any
other problem rather than correcting an inaccurate extack message.
^ permalink raw reply
* Re: [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Eric Dumazet @ 2018-09-27 21:16 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: jiri, jhs, vladbu
In-Reply-To: <20180927204219.17846-1-xiyou.wangcong@gmail.com>
On 09/27/2018 01:42 PM, Cong Wang wrote:
> It is clearly a copy-n-paste.
>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/sched/cls_api.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 3de47e99b788..8dd7f8af6d54 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -655,7 +655,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
>
> *q = qdisc_refcount_inc_nz(*q);
> if (!*q) {
> - NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
> + NL_SET_ERR_MSG(extack, "Can't increase Qdisc refcount");
I am not sure it was a copy-n-paste.
Qdisc refcount business is kernel internal.
If we can not increase the refcount, this is precisely because this qdisc is about
to be destroyed. Nothing fundamentally different than having this thread delayed a bit
and qdisc_lookup_rcu() returning NULL in the first place.
This also means that using RCU for control path is problematic, as surely the caller
of this interface would prefer something that succeeds, even if this means
waiting a bit in the kernel.
Or are we willing to change ip command and make it restart failed syscalls ?
^ permalink raw reply
* [PATCH net] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-09-27 21:08 UTC (permalink / raw)
To: joel, sam
Cc: amithash, vijaykhemka, linux-aspeed, openbmc, sdasari, netdev,
christian
The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
to send NC-SI command to the network card.
Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
The work flow is as below.
Request:
User space application -> Netlink interface (msg)
-> new Netlink handler - ncsi_send_cmd_nl()
-> ncsi_xmit_cmd()
Response:
Response received - ncsi_rcv_rsp() -> internal response handler - ncsi_rsp_handler_xxx()
-> ncsi_rsp_handler_netlink()
-> ncsi_send_netlink_rsp ()
-> Netlink interface (msg)
-> user space application
Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout ()
-> Netlink interface (msg with zero data length)
-> user space application
Error:
Error detected -> ncsi_send_netlink_err () -> Netlink interface (err msg)
-> user space application
Signed-off-by: Justin Lee <justin.lee1@dell.com>
---
include/uapi/linux/ncsi.h | 3 +
net/ncsi/internal.h | 12 ++-
net/ncsi/ncsi-aen.c | 10 ++-
net/ncsi/ncsi-cmd.c | 106 ++++++++++++++++--------
net/ncsi/ncsi-manage.c | 74 ++++++++++++++---
net/ncsi/ncsi-netlink.c | 199 +++++++++++++++++++++++++++++++++++++++++++++-
net/ncsi/ncsi-netlink.h | 4 +
net/ncsi/ncsi-rsp.c | 70 ++++++++++++++--
8 files changed, 420 insertions(+), 58 deletions(-)
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
index 4c292ec..4992bfc 100644
--- a/include/uapi/linux/ncsi.h
+++ b/include/uapi/linux/ncsi.h
@@ -30,6 +30,7 @@ enum ncsi_nl_commands {
NCSI_CMD_PKG_INFO,
NCSI_CMD_SET_INTERFACE,
NCSI_CMD_CLEAR_INTERFACE,
+ NCSI_CMD_SEND_CMD,
__NCSI_CMD_AFTER_LAST,
NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
@@ -43,6 +44,7 @@ enum ncsi_nl_commands {
* @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
* @NCSI_ATTR_PACKAGE_ID: package ID
* @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_DATA: command payload
* @NCSI_ATTR_MAX: highest attribute number
*/
enum ncsi_nl_attrs {
@@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
NCSI_ATTR_PACKAGE_LIST,
NCSI_ATTR_PACKAGE_ID,
NCSI_ATTR_CHANNEL_ID,
+ NCSI_ATTR_DATA,
__NCSI_ATTR_AFTER_LAST,
NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8055e39..20ce735 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -215,12 +215,17 @@ struct ncsi_request {
unsigned char id; /* Request ID - 0 to 255 */
bool used; /* Request that has been assigned */
unsigned int flags; /* NCSI request property */
-#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
struct ncsi_dev_priv *ndp; /* Associated NCSI device */
struct sk_buff *cmd; /* Associated NCSI command packet */
struct sk_buff *rsp; /* Associated NCSI response packet */
struct timer_list timer; /* Timer on waiting for response */
bool enabled; /* Time has been enabled or not */
+
+ u32 snd_seq; /* netlink sending sequence number */
+ u32 snd_portid; /* netlink portid of sender */
+ struct nlmsghdr nlhdr; /* netlink message header */
};
enum {
@@ -301,10 +306,13 @@ struct ncsi_cmd_arg {
unsigned short payload; /* Command packet payload length */
unsigned int req_flags; /* NCSI request properties */
union {
- unsigned char bytes[16]; /* Command packet specific data */
+ unsigned char bytes[16]; /* Command packet specific data */
unsigned short words[8];
unsigned int dwords[4];
};
+
+ unsigned char *data; /* Netlink data */
+ struct genl_info *info; /* Netlink information */
};
extern struct list_head ncsi_dev_list;
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index 25e483e..b5ec193 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -16,6 +16,7 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -73,8 +74,8 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
ncm->data[2] = data;
ncm->data[4] = ntohl(lsc->oem_status);
- netdev_dbg(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
- nc->id, data & 0x1 ? "up" : "down");
+ netdev_dbg(ndp->ndev.dev, "NCSI: LSC AEN - pkg %u ch %u state %s\n",
+ nc->package->id, nc->id, data & 0x1 ? "up" : "down");
chained = !list_empty(&nc->link);
state = nc->state;
@@ -148,9 +149,10 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
spin_unlock_irqrestore(&nc->lock, flags);
+
netdev_dbg(ndp->ndev.dev,
- "NCSI: host driver %srunning on channel %u\n",
- ncm->data[3] & 0x1 ? "" : "not ", nc->id);
+ "NCSI: host driver %srunning on pkg %u ch %u\n",
+ ncm->data[3] & 0x1 ? "" : "not ", nc->package->id, nc->id);
return 0;
}
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 7567ca63..b291297 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -17,6 +17,7 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -211,42 +212,75 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
return 0;
}
+static int ncsi_cmd_handler_oem(struct sk_buff *skb,
+ struct ncsi_cmd_arg *nca)
+{
+ struct ncsi_cmd_pkt *cmd;
+ unsigned char *dest, *source;
+ unsigned short len;
+
+ /* struct ncsi_cmd_pkt = minimum length
+ * - frame checksum
+ * - Ethernet header
+ * = 64 - 4 - 14 = 46
+ * minimum payload = 46 - ncsi header - ncsi checksum
+ * = 46 - 16 - 4 = 26
+ */
+ len = nca->payload;
+
+ /* minimum payload length is 26 bytes to meet minimum packet
+ * length 64
+ */
+ if (len < 26)
+ cmd = skb_put_zero(skb, sizeof(*cmd));
+ else
+ cmd = skb_put_zero(skb, len + sizeof(struct ncsi_pkt_hdr) + 4);
+
+ dest = (unsigned char *)cmd + sizeof(struct ncsi_pkt_hdr);
+ source = (unsigned char *)nca->data + sizeof(struct ncsi_pkt_hdr);
+ memcpy(dest, source, len);
+
+ ncsi_cmd_build_header(&cmd->cmd.common, nca);
+
+ return 0;
+}
+
static struct ncsi_cmd_handler {
unsigned char type;
int payload;
int (*handler)(struct sk_buff *skb,
struct ncsi_cmd_arg *nca);
} ncsi_cmd_handlers[] = {
- { NCSI_PKT_CMD_CIS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SP, 4, ncsi_cmd_handler_sp },
- { NCSI_PKT_CMD_DP, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_EC, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_DC, 4, ncsi_cmd_handler_dc },
- { NCSI_PKT_CMD_RC, 4, ncsi_cmd_handler_rc },
- { NCSI_PKT_CMD_ECNT, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_DCNT, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
- { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
- { NCSI_PKT_CMD_GLS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SVF, 8, ncsi_cmd_handler_svf },
- { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
- { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SMA, 8, ncsi_cmd_handler_sma },
- { NCSI_PKT_CMD_EBF, 4, ncsi_cmd_handler_ebf },
- { NCSI_PKT_CMD_DBF, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_EGMF, 4, ncsi_cmd_handler_egmf },
- { NCSI_PKT_CMD_DGMF, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SNFC, 4, ncsi_cmd_handler_snfc },
- { NCSI_PKT_CMD_GVI, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_GC, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_GP, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_GCPS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_OEM, 0, NULL },
- { NCSI_PKT_CMD_PLDM, 0, NULL },
- { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
+ { NCSI_PKT_CMD_CIS, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_SP, 4, ncsi_cmd_handler_sp },
+ { NCSI_PKT_CMD_DP, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_EC, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_DC, 4, ncsi_cmd_handler_dc },
+ { NCSI_PKT_CMD_RC, 4, ncsi_cmd_handler_rc },
+ { NCSI_PKT_CMD_ECNT, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_DCNT, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
+ { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
+ { NCSI_PKT_CMD_GLS, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_SVF, 8, ncsi_cmd_handler_svf },
+ { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
+ { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_SMA, 8, ncsi_cmd_handler_sma },
+ { NCSI_PKT_CMD_EBF, 4, ncsi_cmd_handler_ebf },
+ { NCSI_PKT_CMD_DBF, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_EGMF, 4, ncsi_cmd_handler_egmf },
+ { NCSI_PKT_CMD_DGMF, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_SNFC, 4, ncsi_cmd_handler_snfc },
+ { NCSI_PKT_CMD_GVI, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_GC, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_GP, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_GCPS, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
+ { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
+ { NCSI_PKT_CMD_PLDM, 0, NULL },
+ { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
};
static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca)
@@ -317,11 +351,20 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
}
/* Get packet payload length and allocate the request */
- nca->payload = nch->payload;
+ if (nch->payload >= 0)
+ nca->payload = nch->payload;
+
nr = ncsi_alloc_command(nca);
if (!nr)
return -ENOMEM;
+ /* track netlink information */
+ if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ nr->snd_seq = nca->info->snd_seq;
+ nr->snd_portid = nca->info->snd_portid;
+ nr->nlhdr = *nca->info->nlhdr;
+ }
+
/* Prepare the packet */
nca->id = nr->id;
ret = nch->handler(nr->cmd, nca);
@@ -341,6 +384,7 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
* connection a 1 second delay should be sufficient.
*/
nr->enabled = true;
+
mod_timer(&nr->timer, jiffies + 1 * HZ);
/* Send NCSI packet */
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0912847..6629103 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -19,6 +19,7 @@
#include <net/addrconf.h>
#include <net/ipv6.h>
#include <net/if_inet6.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -110,8 +111,9 @@ static void ncsi_channel_monitor(struct timer_list *t)
case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
break;
default:
- netdev_err(ndp->ndev.dev, "NCSI Channel %d timed out!\n",
- nc->id);
+ netdev_err(ndp->ndev.dev, "NCSI: pkg %u ch %u timed out!\n",
+ np->id, nc->id);
+
if (!(ndp->flags & NCSI_DEV_HWA)) {
ncsi_report_link(ndp, true);
ndp->flags |= NCSI_DEV_RESHUFFLE;
@@ -143,6 +145,10 @@ void ncsi_start_channel_monitor(struct ncsi_channel *nc)
{
unsigned long flags;
+ netdev_dbg(nc->package->ndp->ndev.dev,
+ "NCSI: %s pkg %u ch %u\n",
+ __func__, nc->package->id, nc->id);
+
spin_lock_irqsave(&nc->lock, flags);
WARN_ON_ONCE(nc->monitor.enabled);
nc->monitor.enabled = true;
@@ -156,6 +162,10 @@ void ncsi_stop_channel_monitor(struct ncsi_channel *nc)
{
unsigned long flags;
+ netdev_dbg(nc->package->ndp->ndev.dev,
+ "NCSI: %s pkg %u ch %u\n",
+ __func__, nc->package->id, nc->id);
+
spin_lock_irqsave(&nc->lock, flags);
if (!nc->monitor.enabled) {
spin_unlock_irqrestore(&nc->lock, flags);
@@ -406,8 +416,13 @@ static void ncsi_request_timeout(struct timer_list *t)
{
struct ncsi_request *nr = from_timer(nr, t, timer);
struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ struct ncsi_cmd_pkt *cmd;
unsigned long flags;
+ netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
+
/* If the request already had associated response,
* let the response handler to release it.
*/
@@ -415,10 +430,23 @@ static void ncsi_request_timeout(struct timer_list *t)
nr->enabled = false;
if (nr->rsp || !nr->cmd) {
spin_unlock_irqrestore(&ndp->lock, flags);
+
+ netdev_dbg(ndp->ndev.dev, "NCSI: %s - early return\n", __func__);
+
return;
}
spin_unlock_irqrestore(&ndp->lock, flags);
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (nr->cmd) {
+ /* Find the package */
+ cmd = (struct ncsi_cmd_pkt *)skb_network_header(nr->cmd);
+ ncsi_find_package_and_channel(ndp, cmd->cmd.common.channel,
+ &np, &nc);
+ ncsi_send_netlink_timeout(nr, np, nc);
+ }
+ }
+
/* Release the request */
ncsi_free_request(nr);
}
@@ -432,6 +460,10 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
unsigned long flags;
int ret;
+ netdev_dbg(ndp->ndev.dev,
+ "NCSI: %s pkg %u ch %u state %04x\n",
+ __func__, np->id, nc->id, nd->state);
+
nca.ndp = ndp;
nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
@@ -647,6 +679,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
unsigned long flags;
int ret;
+ netdev_dbg(ndp->ndev.dev,
+ "NCSI: %s pkg %u ch %u state %04x\n",
+ __func__, np->id, nc->id, nd->state);
+
nca.ndp = ndp;
nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
@@ -788,8 +824,9 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
}
break;
case ncsi_dev_state_config_done:
- netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
- nc->id);
+ netdev_dbg(ndp->ndev.dev,
+ "NCSI: pkg %u ch %u config done\n",
+ nc->package->id, nc->id);
spin_lock_irqsave(&nc->lock, flags);
if (nc->reconfigure_needed) {
/* This channel's configuration has been updated
@@ -815,9 +852,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
} else {
hot_nc = NULL;
nc->state = NCSI_CHANNEL_INACTIVE;
+
netdev_dbg(ndp->ndev.dev,
- "NCSI: channel %u link down after config\n",
- nc->id);
+ "NCSI: pkg %u ch %u link down after config\n",
+ nc->package->id, nc->id);
}
spin_unlock_irqrestore(&nc->lock, flags);
@@ -853,6 +891,8 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
force_package = ndp->force_package;
spin_unlock_irqrestore(&ndp->lock, flags);
+ netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
+
/* Force a specific channel whether or not it has link if we have been
* configured to do so
*/
@@ -861,8 +901,8 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
ncm = &found->modes[NCSI_MODE_LINK];
if (!(ncm->data[2] & 0x1))
netdev_info(ndp->ndev.dev,
- "NCSI: Channel %u forced, but it is link down\n",
- found->id);
+ "NCSI: pkg %u ch %u forced, but it is link down\n",
+ found->package->id, found->id);
goto out;
}
@@ -914,6 +954,7 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
out:
spin_lock_irqsave(&ndp->lock, flags);
list_add_tail_rcu(&found->link, &ndp->channel_queue);
+
spin_unlock_irqrestore(&ndp->lock, flags);
return ncsi_process_next_channel(ndp);
@@ -1154,6 +1195,8 @@ static void ncsi_dev_work(struct work_struct *work)
struct ncsi_dev_priv, work);
struct ncsi_dev *nd = &ndp->ndev;
+ netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
+
switch (nd->state & ncsi_dev_state_major) {
case ncsi_dev_state_probe:
ncsi_probe_channel(ndp);
@@ -1176,6 +1219,8 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
int old_state;
unsigned long flags;
+ netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
+
spin_lock_irqsave(&ndp->lock, flags);
nc = list_first_or_null_rcu(&ndp->channel_queue,
struct ncsi_channel, link);
@@ -1198,14 +1243,14 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
switch (old_state) {
case NCSI_CHANNEL_INACTIVE:
ndp->ndev.state = ncsi_dev_state_config;
- netdev_dbg(ndp->ndev.dev, "NCSI: configuring channel %u\n",
- nc->id);
+ netdev_dbg(ndp->ndev.dev, "NCSI: configuring pkg %u ch %u\n",
+ nc->package->id, nc->id);
ncsi_configure_channel(ndp);
break;
case NCSI_CHANNEL_ACTIVE:
ndp->ndev.state = ncsi_dev_state_suspend;
- netdev_dbg(ndp->ndev.dev, "NCSI: suspending channel %u\n",
- nc->id);
+ netdev_dbg(ndp->ndev.dev, "NCSI: suspending pkg %u ch %u\n",
+ nc->package->id, nc->id);
ncsi_suspend_channel(ndp);
break;
default:
@@ -1225,6 +1270,9 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
return ncsi_choose_active_channel(ndp);
}
+ netdev_dbg(ndp->ndev.dev,
+ "NCSI: No more channels to process\n");
+
ncsi_report_link(ndp, false);
return -ENODEV;
}
@@ -1475,6 +1523,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
if (list_empty(&ncsi_dev_list))
register_inet6addr_notifier(&ncsi_inet6addr_notifier);
#endif
+
list_add_tail_rcu(&ndp->node, &ncsi_dev_list);
spin_unlock_irqrestore(&ncsi_dev_lock, flags);
@@ -1564,6 +1613,7 @@ void ncsi_unregister_dev(struct ncsi_dev *nd)
if (list_empty(&ncsi_dev_list))
unregister_inet6addr_notifier(&ncsi_inet6addr_notifier);
#endif
+
spin_unlock_irqrestore(&ncsi_dev_lock, flags);
ncsi_unregister_netlink(nd->dev);
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 45f33d6..ab1a959 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -20,6 +20,7 @@
#include <uapi/linux/ncsi.h>
#include "internal.h"
+#include "ncsi-pkt.h"
#include "ncsi-netlink.h"
static struct genl_family ncsi_genl_family;
@@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
[NCSI_ATTR_PACKAGE_LIST] = { .type = NLA_NESTED },
[NCSI_ATTR_PACKAGE_ID] = { .type = NLA_U32 },
[NCSI_ATTR_CHANNEL_ID] = { .type = NLA_U32 },
+ [NCSI_ATTR_DATA] = { .type = NLA_BINARY, .len = 2048 },
};
static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
@@ -240,7 +242,7 @@ static int ncsi_pkg_info_all_nl(struct sk_buff *skb,
return 0; /* done */
hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
- &ncsi_genl_family, NLM_F_MULTI, NCSI_CMD_PKG_INFO);
+ &ncsi_genl_family, NLM_F_MULTI, NCSI_CMD_PKG_INFO);
if (!hdr) {
rc = -EMSGSIZE;
goto err;
@@ -316,8 +318,8 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info)
* package
*/
spin_unlock_irqrestore(&ndp->lock, flags);
- netdev_info(ndp->ndev.dev, "NCSI: Channel %u does not exist!\n",
- channel_id);
+ netdev_info(ndp->ndev.dev, "NCSI: pkg %u ch %u does not exist!\n",
+ package_id, channel_id);
return -ERANGE;
}
@@ -366,6 +368,191 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
return 0;
}
+static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
+{
+ struct ncsi_dev_priv *ndp;
+
+ struct ncsi_cmd_arg nca;
+ struct ncsi_pkt_hdr *hdr;
+
+ u32 package_id, channel_id;
+ unsigned char *data;
+ void *head;
+ int len, ret;
+
+ if (!info || !info->attrs) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_IFINDEX]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
+ nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+ if (!ndp) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
+ channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
+
+ if ((package_id & ~0x07) || (channel_id & ~0x1f)) {
+ ret = -ERANGE;
+ goto out_netlink;
+ }
+
+ len = nla_len(info->attrs[NCSI_ATTR_DATA]);
+ if (len < sizeof(struct ncsi_pkt_hdr)) {
+ netdev_info(ndp->ndev.dev, "NCSI: no OEM command to send %u\n",
+ package_id);
+ ret = -EINVAL;
+ goto out_netlink;
+ } else {
+ head = nla_data(info->attrs[NCSI_ATTR_DATA]);
+ data = (unsigned char *)head;
+ }
+
+ hdr = (struct ncsi_pkt_hdr *)data;
+
+ nca.ndp = ndp;
+ nca.package = (unsigned char)package_id;
+ nca.channel = (unsigned char)channel_id;
+ nca.type = hdr->type;
+ nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
+ nca.info = info;
+ nca.payload = ntohs(hdr->length);
+ nca.data = data;
+
+ ret = ncsi_xmit_cmd(&nca);
+out_netlink:
+ if (ret != 0) {
+ netdev_err(ndp->ndev.dev, "Error %d sending OEM command\n", ret);
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ info->snd_seq, info->snd_portid, info->nlhdr,
+ ret);
+ }
+out:
+ return ret;
+}
+
+int ncsi_send_netlink_rsp(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+ int rc;
+
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
+
+ net = dev_net(nr->rsp->dev);
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
+ if (rc)
+ goto err;
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+
+err:
+ kfree_skb(skb);
+ return rc;
+}
+
+int ncsi_send_netlink_timeout(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ net = dev_net(nr->cmd->dev);
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
+
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
+ NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)nr->cmd->data)->channel)));
+
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+}
+
+int ncsi_send_netlink_err(struct net_device *dev, u32 snd_seq, u32 snd_portid, struct nlmsghdr *nlhdr, int err)
+{
+ struct sk_buff *skb;
+ struct nlmsghdr *nlh;
+ struct nlmsgerr *nle;
+ struct net *net;
+
+ skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ net = dev_net(dev);
+
+ nlh = nlmsg_put(skb, snd_portid, snd_seq,
+ NLMSG_ERROR, sizeof(*nle), 0);
+ nle = (struct nlmsgerr *)nlmsg_data(nlh);
+ nle->error = err;
+ memcpy(&nle->msg, nlhdr, sizeof(*nlh));
+
+ nlmsg_end(skb, nlh);
+
+ return nlmsg_unicast(net->genl_sock, skb, snd_portid);
+}
+
static const struct genl_ops ncsi_ops[] = {
{
.cmd = NCSI_CMD_PKG_INFO,
@@ -386,6 +573,12 @@ static const struct genl_ops ncsi_ops[] = {
.doit = ncsi_clear_interface_nl,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NCSI_CMD_SEND_CMD,
+ .policy = ncsi_genl_policy,
+ .doit = ncsi_send_cmd_nl,
+ .flags = GENL_ADMIN_PERM,
+ },
};
static struct genl_family ncsi_genl_family __ro_after_init = {
diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
index 91a5c25..dadaf32 100644
--- a/net/ncsi/ncsi-netlink.h
+++ b/net/ncsi/ncsi-netlink.h
@@ -14,6 +14,10 @@
#include "internal.h"
+int ncsi_send_netlink_rsp(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc);
+int ncsi_send_netlink_timeout(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc);
+int ncsi_send_netlink_err(struct net_device *dev, u32 snd_seq, u32 snd_portid, struct nlmsghdr *nlhdr, int err);
+
int ncsi_init_netlink(struct net_device *dev);
int ncsi_unregister_netlink(struct net_device *dev);
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 930c1d3..bdf9519 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -16,9 +16,11 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
+#include "ncsi-netlink.h"
static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
unsigned short payload)
@@ -32,15 +34,22 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
* before calling this function.
*/
h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
- if (h->common.revision != NCSI_PKT_REVISION)
+
+ if (h->common.revision != NCSI_PKT_REVISION) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: unsupported header revision\n");
return -EINVAL;
- if (ntohs(h->common.length) != payload)
+ }
+ if (ntohs(h->common.length) != payload) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: payload length mismatched\n");
return -EINVAL;
+ }
/* Check on code and reason */
if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
- ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
- return -EINVAL;
+ ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: non zero response/reason code\n");
+ return -EPERM;
+ }
/* Validate checksum, which might be zeroes if the
* sender doesn't support checksum according to NCSI
@@ -52,8 +61,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
checksum = ncsi_calculate_checksum((unsigned char *)h,
sizeof(*h) + payload - 4);
- if (*pchecksum != htonl(checksum))
+
+ if (*pchecksum != htonl(checksum)) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
return -EINVAL;
+ }
return 0;
}
@@ -900,6 +912,31 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
return 0;
}
+static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
+{
+ return 0;
+}
+
+static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_pkt *rsp;
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ int ret;
+
+ /* Find the package */
+ rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
+ ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
+ &np, &nc);
+ if (!np)
+ return -ENODEV;
+
+ ret = ncsi_send_netlink_rsp(nr, np, nc);
+
+ return ret;
+}
+
static struct ncsi_rsp_handler {
unsigned char type;
int payload;
@@ -932,7 +969,7 @@ static struct ncsi_rsp_handler {
{ NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
{ NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
{ NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
- { NCSI_PKT_RSP_OEM, 0, NULL },
+ { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
{ NCSI_PKT_RSP_PLDM, 0, NULL },
{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
};
@@ -950,6 +987,7 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
/* Find the NCSI device */
nd = ncsi_find_dev(dev);
+
ndp = nd ? TO_NCSI_DEV_PRIV(nd) : NULL;
if (!ndp)
return -ENODEV;
@@ -1002,6 +1040,15 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_warn(ndp->ndev.dev,
"NCSI: 'bad' packet ignored for type 0x%x\n",
hdr->type);
+
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (ret == -EPERM)
+ goto out_netlink;
+ else
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ nr->snd_seq, nr->snd_portid, &nr->nlhdr,
+ ret);
+ }
goto out;
}
@@ -1011,6 +1058,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_err(ndp->ndev.dev,
"NCSI: Handler for packet type 0x%x returned %d\n",
hdr->type, ret);
+
+out_netlink:
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ ret = ncsi_rsp_handler_netlink(nr);
+ if (ret) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Netlink handler for packet type 0x%x returned %d\n",
+ hdr->type, ret);
+ }
+ }
+
out:
ncsi_free_request(nr);
return ret;
--
2.9.3
^ permalink raw reply related
* [Patch net-next] net_sched: fix a crash in tc_new_tfilter()
From: Cong Wang @ 2018-09-27 20:42 UTC (permalink / raw)
To: netdev; +Cc: jiri, jhs, vladbu, Cong Wang
In-Reply-To: <20180927204219.17846-1-xiyou.wangcong@gmail.com>
When tcf_block_find() fails, it already rollbacks the qdisc refcnt,
so its caller doesn't need to clean up this again. Avoid calling
qdisc_put() again by resetting qdisc to NULL for callers.
Reported-by: syzbot+37b8770e6d5a8220a039@syzkaller.appspotmail.com
Fixes: e368fdb61d8e ("net: sched: use Qdisc rcu API instead of relying on rtnl lock")
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 8dd7f8af6d54..a4167ec0a220 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -717,8 +717,10 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
errout_rcu:
rcu_read_unlock();
errout_qdisc:
- if (*q)
+ if (*q) {
qdisc_put(*q);
+ *q = NULL;
+ }
return ERR_PTR(err);
}
--
2.14.4
^ permalink raw reply related
* [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Cong Wang @ 2018-09-27 20:42 UTC (permalink / raw)
To: netdev; +Cc: jiri, jhs, vladbu, Cong Wang
It is clearly a copy-n-paste.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 3de47e99b788..8dd7f8af6d54 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -655,7 +655,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
*q = qdisc_refcount_inc_nz(*q);
if (!*q) {
- NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
+ NL_SET_ERR_MSG(extack, "Can't increase Qdisc refcount");
err = -EINVAL;
goto errout_rcu;
}
--
2.14.4
^ permalink raw reply related
* Re: [PATCH net-next 0/7] rtnetlink: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 20:31 UTC (permalink / raw)
To: David Ahern, jbenc, davem, stephen, netdev, linux-kernel
In-Reply-To: <e7ec6202-7d4b-8dc1-1b25-76325129ba6f@gmail.com>
On September 27, 2018 10:24:36 PM GMT+02:00, David Ahern <dsahern@gmail.com> wrote:
>On 9/27/18 11:58 AM, Christian Brauner wrote:
>> Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
>> requests with struct ifinfomsg. This is wrong and should have been
>> struct ifaddrmsg all along as mandated by the manpages. However, dump
>> requests so far didn't parse the netlink message that was sent and
>> succeeded even when a wrong struct was passed along.
>
>...
>
>> The correct solution at this point seems to me to introduce a new
>> RTM_GETADDR2 request. This way we can parse the message and fail hard
>if
>> the struct is not struct ifaddrmsg and can safely extend it in the
>> future. Userspace tools that rely on the buggy RTM_GETADDR API will
>> still keep working without even having to see any log messages and
>new
>> userspace tools that want to make user of new features can make use
>of
>> the new RTM_GETADDR2 requests.
>
>First, I think this is the wrong precedent when all we need is a single
>bit flag that userspace can use to tell the kernel "I have a clue and I
>am passing in the proper header for this dump request".
That had been NAKed previously but if you have an idea that will be accepted all the more power to you.
>
>Second, you are not addressing the problems of the past by requiring
>the
>proper header and checking values passed in it.
I don't follow. RTM_GETADDR requests are absolutely unchanged. The full legacy behavior is restored by this patchset.
And requiring that RTM_GETADDR2 requests always pass the correct header is absolutely fine. We don't want built invalid legacy behavior into a new request type.
>
>I have another idea. I'll send an RFC patch soon.
^ permalink raw reply
* RE: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Chris Preimesberger @ 2018-09-27 20:17 UTC (permalink / raw)
To: 'Andrew Lunn', 'Eran Ben Elisha'
Cc: 'Neil Horman', 'linville@tuxdriver.com',
'netdev@vger.kernel.org'
In-Reply-To: <82CEAF9FFBA4DD428B132074FB91DF7D5F648BA5@CSI-MAILSRV.csicompanies.internal>
Update for posterity-
Mellanox support provided a work-around of using mlxcables instead of
ethtool to read alarm/warning info for an installed transceiver.
I was told that a couple of their engineers are currently looking into the
discrepancy between threshold reporting by mlxcables and ethtool, and
that they are deciding what to do about it...
Work-around steps:
1. add a cable with "sudo mst cable add".
2. find the cable name with "sudo mlxcables". The name of my cable is
01:00.0_cable_0 so I copy that name for insertion into the next command.
3. probe the cable for DDM with "sudo mlxcables -d 01:00.0_cable_0 --DDM".
Example copied/pasted from my CLI here.
All reported thresholds appear to be correct.
tech1@D7:~$
tech1@D7:~$
tech1@D7:~$ sudo mst cable add
-I- Added 1 cable devices ..
tech1@D7:~$ sudo mlxcables
Querying Cables ....
Cable #1:
---------
Cable name : 01:00.0_cable_0
>> No FW data to show
-------- Cable EEPROM --------
Identifier : QSFP28 (11h)
Technology : 850 nm VCSEL (00h)
Compliance : Extended Specification Compliance is valid, 100GBASE-SR4 or 25GBASE-SR
Wavelength : 850 nm
OUI : 0x00c0f2
Vendor : TRANSITION
Serial number : TN02000263
Part number : TN-QSFP-100G-SR4
Revision : 02
Temperature : 34 C
Length : 50 m
tech1@D7:~$ sudo mlxcables -d 01:00.0_cable_0 --DDM
Cable DDM:
----------
Temperature : 34C
Voltage : 3.2918V
Channel 1:
RX Power : 0.1695dBm
TX Power : 0.8622dBm
TX Bias : 7.0720mA
Channel 2:
RX Power : 0.1355dBm
TX Power : 1.1042dBm
TX Bias : 6.9240mA
Channel 3:
RX Power : -0.1592dBm
TX Power : 0.6547dBm
TX Bias : 6.9420mA
Channel 4:
RX Power : -0.1300dBm
TX Power : 0.4653dBm
TX Bias : 6.9120mA
----- Thresholds -----
Temperature:
High Warning : 70C
Low Warning : 0C
High Alarm : 75C
Low Alarm : -5C
Warning mask : 0
Alarm mask : 0
Voltage:
High Warning : 3.4600V
Low Warning : 3.1300V
High Alarm : 3.6300V
Low Alarm : 2.9700V
Warning mask : 0
Alarm mask : 0
Channel 1:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
Channel 2:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
Channel 3:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
Channel 4:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
tech1@D7:~$
tech1@D7:~$
tech1@D7:~$
Chris Preimesberger
^ permalink raw reply
* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-09-28 2:35 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-kernel, netdev, linux-crypto, davem, gregkh
In-Reply-To: <20180928011726.GA98113@gmail.com>
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.
> 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.
> 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. 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.
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.
> > - 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.
> 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.
Jason
^ permalink raw reply
* Re: [PATCH net-next] tcp: up initial rmem to 128KB and SYN rwin to around 64KB
From: Yuchung Cheng @ 2018-09-27 19:39 UTC (permalink / raw)
To: David Miller, Eric Dumazet
Cc: netdev, Neal Cardwell, Wei Wang, Soheil Hassas Yeganeh,
Yuchung Cheng
In-Reply-To: <20180927182119.161132-1-ycheng@google.com>
On Thu, Sep 27, 2018 at 11:21 AM, Yuchung Cheng <ycheng@google.com> wrote:
> Previously TCP initial receive buffer is ~87KB by default and
> the initial receive window is ~29KB (20 MSS). This patch changes
> the two numbers to 128KB and ~64KB (rounding down to the multiples
> of MSS) respectively. The patch also simplifies the calculations s.t.
> the two numbers are directly controlled by sysctl tcp_rmem[1]:
>
> 1) Initial receiver buffer budget (sk_rcvbuf): while this should
> be configured via sysctl tcp_rmem[1], previously tcp_fixup_rcvbuf()
> always override and set a larger size when a new connection
> establishes.
>
> 2) Initial receive window in SYN: previously it is set to 20
> packets if MSS <= 1460. The number 20 was based on the initial
> congestion window of 10: the receiver needs twice amount to
> avoid being limited by the receive window upon out-of-order
> delivery in the first window burst. But since this only
> applies if the receiving MSS <= 1460, connection using large MTU
> (e.g. to utilize receiver zero-copy) may be limited by the
> receive window.
>
> With this patch TCP memory configuration is more straight-forward and
> more properly sized to modern high-speed networks by default. Several
> popular stacks have been announcing 64KB rwin in SYNs as well.
Sorry please ignore this patch for now.
We need to adjust rbuf autotuning as well otherwise w/ larger init
rbuf it may increase too slowly during slow start. Will submit a v2
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> net/ipv4/tcp.c | 4 ++--
> net/ipv4/tcp_input.c | 25 ++-----------------------
> net/ipv4/tcp_output.c | 25 ++++---------------------
> 3 files changed, 8 insertions(+), 46 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 69c236943f56..dcf51fbf5ec7 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3896,8 +3896,8 @@ void __init tcp_init(void)
> init_net.ipv4.sysctl_tcp_wmem[2] = max(64*1024, max_wshare);
>
> init_net.ipv4.sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
> - init_net.ipv4.sysctl_tcp_rmem[1] = 87380;
> - init_net.ipv4.sysctl_tcp_rmem[2] = max(87380, max_rshare);
> + init_net.ipv4.sysctl_tcp_rmem[1] = 131072;
> + init_net.ipv4.sysctl_tcp_rmem[2] = max(131072, max_rshare);
>
> pr_info("Hash tables configured (established %u bind %u)\n",
> tcp_hashinfo.ehash_mask + 1, tcp_hashinfo.bhash_size);
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index d703a0b3b6a2..7a59f6a96212 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -426,26 +426,7 @@ static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb)
> }
> }
>
> -/* 3. Tuning rcvbuf, when connection enters established state. */
> -static void tcp_fixup_rcvbuf(struct sock *sk)
> -{
> - u32 mss = tcp_sk(sk)->advmss;
> - int rcvmem;
> -
> - rcvmem = 2 * SKB_TRUESIZE(mss + MAX_TCP_HEADER) *
> - tcp_default_init_rwnd(mss);
> -
> - /* Dynamic Right Sizing (DRS) has 2 to 3 RTT latency
> - * Allow enough cushion so that sender is not limited by our window
> - */
> - if (sock_net(sk)->ipv4.sysctl_tcp_moderate_rcvbuf)
> - rcvmem <<= 2;
> -
> - if (sk->sk_rcvbuf < rcvmem)
> - sk->sk_rcvbuf = min(rcvmem, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]);
> -}
> -
> -/* 4. Try to fixup all. It is made immediately after connection enters
> +/* 3. Try to fixup all. It is made immediately after connection enters
> * established state.
> */
> void tcp_init_buffer_space(struct sock *sk)
> @@ -454,8 +435,6 @@ void tcp_init_buffer_space(struct sock *sk)
> struct tcp_sock *tp = tcp_sk(sk);
> int maxwin;
>
> - if (!(sk->sk_userlocks & SOCK_RCVBUF_LOCK))
> - tcp_fixup_rcvbuf(sk);
> if (!(sk->sk_userlocks & SOCK_SNDBUF_LOCK))
> tcp_sndbuf_expand(sk);
>
> @@ -485,7 +464,7 @@ void tcp_init_buffer_space(struct sock *sk)
> tp->snd_cwnd_stamp = tcp_jiffies32;
> }
>
> -/* 5. Recalculate window clamp after socket hit its memory bounds. */
> +/* 4. Recalculate window clamp after socket hit its memory bounds. */
> static void tcp_clamp_window(struct sock *sk)
> {
> struct tcp_sock *tp = tcp_sk(sk);
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index fe7855b090e4..059b67af28b1 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -195,21 +195,6 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts,
> inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
> }
>
> -
> -u32 tcp_default_init_rwnd(u32 mss)
> -{
> - /* Initial receive window should be twice of TCP_INIT_CWND to
> - * enable proper sending of new unsent data during fast recovery
> - * (RFC 3517, Section 4, NextSeg() rule (2)). Further place a
> - * limit when mss is larger than 1460.
> - */
> - u32 init_rwnd = TCP_INIT_CWND * 2;
> -
> - if (mss > 1460)
> - init_rwnd = max((1460 * init_rwnd) / mss, 2U);
> - return init_rwnd;
> -}
> -
> /* Determine a window scaling and initial window to offer.
> * Based on the assumption that the given amount of space
> * will be offered. Store the results in the tp structure.
> @@ -244,7 +229,10 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
> if (sock_net(sk)->ipv4.sysctl_tcp_workaround_signed_windows)
> (*rcv_wnd) = min(space, MAX_TCP_WINDOW);
> else
> - (*rcv_wnd) = space;
> + (*rcv_wnd) = min_t(u32, space, U16_MAX);
> +
> + if (init_rcv_wnd)
> + *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
>
> (*rcv_wscale) = 0;
> if (wscale_ok) {
> @@ -257,11 +245,6 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
> (*rcv_wscale)++;
> }
> }
> -
> - if (!init_rcv_wnd) /* Use default unless specified otherwise */
> - init_rcv_wnd = tcp_default_init_rwnd(mss);
> - *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
> -
> /* Set the clamp no higher than max representable value */
> (*window_clamp) = min_t(__u32, U16_MAX << (*rcv_wscale), *window_clamp);
> }
> --
> 2.19.0.605.g01d371f741-goog
>
^ permalink raw reply
* [PATCH v2 net-next 1/2] net: phy: micrel: add Microchip KSZ9131 inital driver
From: Yuiko Oshino @ 2018-09-27 20:16 UTC (permalink / raw)
To: davem, robh+dt, devicetree, f.fainelli, andrew
Cc: linux-kernel, mark.rutland, m.felsch, Markus.Niebel, netdev,
UNGLinuxDriver
Add support for Microchip Technology KSZ9131 10/100/1000 Ethernet PHY
Signed-off-by: Yuiko Oshino <yuiko.oshino@microchip.com>
---
drivers/net/phy/micrel.c | 32 +++++++++++++++++++++++++++++---
include/linux/micrel_phy.h | 1 +
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 3db06b4..a4473cb 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -14,7 +14,7 @@
* option) any later version.
*
* Support : Micrel Phys:
- * Giga phys: ksz9021, ksz9031
+ * Giga phys: ksz9021, ksz9031, ksz9131
* 100/10 Phys : ksz8001, ksz8721, ksz8737, ksz8041
* ksz8021, ksz8031, ksz8051,
* ksz8081, ksz8091,
@@ -425,6 +425,7 @@ static int ksz9021_config_init(struct phy_device *phydev)
#define MII_KSZ9031RN_MMD_REGDATA_REG 0x0e
#define OP_DATA 1
#define KSZ9031_PS_TO_REG 60
+#define KSZ9131_PS_TO_REG 100
/* Extended registers */
/* MMD Address 0x0 */
@@ -470,6 +471,10 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
u16 maxval;
u16 newval;
int i;
+ int pstoreg = KSZ9031_PS_TO_REG;
+
+ if (phydev->drv->phy_id == PHY_ID_KSZ9131)
+ pstoreg = KSZ9131_PS_TO_REG;
for (i = 0; i < numfields; i++)
if (!of_property_read_u32(of_node, field[i], val + i))
@@ -489,7 +494,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
mask = 0xffff;
mask ^= maxval << (field_sz * i);
newval = (newval & mask) |
- (((val[i] / KSZ9031_PS_TO_REG) & maxval)
+ (((val[i] / pstoreg) & maxval)
<< (field_sz * i));
}
@@ -602,7 +607,10 @@ static int ksz9031_config_init(struct phy_device *phydev)
}
}
- return ksz9031_center_flp_timing(phydev);
+ if (phydev->drv->phy_id == PHY_ID_KSZ9031)
+ return ksz9031_center_flp_timing(phydev);
+ else
+ return 0;
err_force_master:
phydev_err(phydev, "failed to force the phy to master mode\n");
@@ -975,6 +983,23 @@ static struct phy_driver ksphy_driver[] = {
.suspend = genphy_suspend,
.resume = kszphy_resume,
}, {
+ .phy_id = PHY_ID_KSZ9131,
+ .phy_id_mask = MICREL_PHY_ID_MASK,
+ .name = "Microchip KSZ9131 Gigabit PHY",
+ .features = PHY_GBIT_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .driver_data = &ksz9021_type,
+ .probe = kszphy_probe,
+ .config_init = ksz9031_config_init,
+ .read_status = ksz9031_read_status,
+ .ack_interrupt = kszphy_ack_interrupt,
+ .config_intr = kszphy_config_intr,
+ .get_sset_count = kszphy_get_sset_count,
+ .get_strings = kszphy_get_strings,
+ .get_stats = kszphy_get_stats,
+ .suspend = genphy_suspend,
+ .resume = kszphy_resume,
+}, {
.phy_id = PHY_ID_KSZ8873MLL,
.phy_id_mask = MICREL_PHY_ID_MASK,
.name = "Micrel KSZ8873MLL Switch",
@@ -1022,6 +1047,7 @@ MODULE_LICENSE("GPL");
static struct mdio_device_id __maybe_unused micrel_tbl[] = {
{ PHY_ID_KSZ9021, 0x000ffffe },
{ PHY_ID_KSZ9031, MICREL_PHY_ID_MASK },
+ { PHY_ID_KSZ9131, MICREL_PHY_ID_MASK },
{ PHY_ID_KSZ8001, 0x00fffffc },
{ PHY_ID_KS8737, MICREL_PHY_ID_MASK },
{ PHY_ID_KSZ8021, 0x00ffffff },
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 472fa4d..7361cd3 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -31,6 +31,7 @@
#define PHY_ID_KSZ8081 0x00221560
#define PHY_ID_KSZ8061 0x00221570
#define PHY_ID_KSZ9031 0x00221620
+#define PHY_ID_KSZ9131 0x00221640
#define PHY_ID_KSZ886X 0x00221430
#define PHY_ID_KSZ8863 0x00221435
--
2.7.4
^ permalink raw reply related
* [PATCH v2 net-next 0/2] Add support for Microchip Technology KSZ9131 10/100/1000 Ethernet PHY
From: Yuiko Oshino @ 2018-09-27 20:15 UTC (permalink / raw)
To: davem, robh+dt, devicetree, f.fainelli, andrew
Cc: linux-kernel, mark.rutland, m.felsch, Markus.Niebel, netdev,
UNGLinuxDriver
This is the initial driver for Microchip KSZ9131 10/100/1000 Ethernet PHY
v2:
- Creating a series from two related patches.
Yuiko Oshino (2):
net: phy: micrel: add Microchip KSZ9131 inital driver
dt-bindings: net: add support for Microchip KSZ9131 Ethernet PHY
.../devicetree/bindings/net/micrel-ksz90x1.txt | 29 +++++++++++++++++++-
drivers/net/phy/micrel.c | 32 ++++++++++++++++++++--
include/linux/micrel_phy.h | 1 +
3 files changed, 58 insertions(+), 4 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH bpf-next 0/5] Introduce libbpf_attach_type_by_name
From: Daniel Borkmann @ 2018-09-27 19:21 UTC (permalink / raw)
To: Andrey Ignatov, netdev; +Cc: ast, kernel-team
In-Reply-To: <cover.1538000102.git.rdna@fb.com>
On 09/27/2018 12:24 AM, Andrey Ignatov wrote:
> This patch set introduces libbpf_attach_type_by_name function in libbpf to
> identify attach type by section name.
>
> This is useful to avoid writing same logic over and over again in user
> space applications that leverage libbpf.
>
> Patch 1 has more details on the new function and problem being solved.
> Patches 2 and 3 add support for new section names.
> Patch 4 uses new function in a selftest.
> Patch 5 adds selftest for libbpf_{prog,attach}_type_by_name.
>
> As a side note there are a lot of inconsistencies now between names used by
> libbpf and bpftool (e.g. cgroup/skb vs cgroup_skb, cgroup_device and device
> vs cgroup/dev, sockops vs sock_ops, etc). This patch set does not address
> it but it tries not to make it harder to address it in the future.
>
>
> Andrey Ignatov (5):
> libbpf: Introduce libbpf_attach_type_by_name
> libbpf: Support cgroup_skb/{e,in}gress section names
> libbpf: Support sk_skb/stream_{parser,verdict} section names
> selftests/bpf: Use libbpf_attach_type_by_name in test_socket_cookie
> selftests/bpf: Test libbpf_{prog,attach}_type_by_name
Applied to bpf-next, thanks Andrey!
^ permalink raw reply
* Re: WARN_ON in TLP causing RT throttling
From: Yuchung Cheng @ 2018-09-27 19:14 UTC (permalink / raw)
To: Eric Dumazet; +Cc: stranche, soheil@google.com
In-Reply-To: <3c112d25-1105-524f-e210-8a7cb63df1c7@gmail.com>
On Wed, Sep 26, 2018 at 5:09 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/26/2018 04:46 PM, stranche@codeaurora.org wrote:
> > Hi Eric,
> >
> > Someone recently reported a crash to us on the 4.14.62 kernel where excessive
> > WARNING prints were spamming the logs and causing watchdog bites. The kernel
> > does have the following commit by Soheil:
> > bffd168c3fc5 "tcp: clear tp->packets_out when purging write queue"
> >
> > Before this bug we see over 1 second of continuous WARN_ON prints from
> > tcp_send_loss_probe() like so:
> >
> > 7795.530450: <2> tcp_send_loss_probe+0x194/0x1b8
> > 7795.534833: <2> tcp_write_timer_handler+0xf8/0x1c4
> > 7795.539492: <2> tcp_write_timer+0x4c/0x74
> > 7795.543348: <2> call_timer_fn+0xc0/0x1b4
> > 7795.547113: <2> run_timer_softirq+0x248/0x81c
> >
> > Specifically, the prints come from the following check:
> >
> > /* Retransmit last segment. */
> > if (WARN_ON(!skb))
> > goto rearm_timer;
> >
> > Since skb is always NULL, we know there's nothing on the write queue or the
> > retransmit queue, so we just keep resetting the timer, waiting for more data
> > to be queued. However, we were able to determine that the TCP socket is in the
> > TCP_FIN_WAIT1 state, so we will no longer be sending any data and these queues
> > remain empty.
> >
> > Would it be appropriate to stop resetting the TLP timer if we detect that the
> > connection is starting to close and we have no more data to send the probe with,
> > or is there some way that this scenario should already be handled?
> >
> > Unfortunately, we don't have a reproducer for this crash.
> >
>
> Something is fishy.
>
> If there is no skb in the queues, then tp->packets_out should be 0,
> therefore tcp_rearm_rto() should simply call inet_csk_clear_xmit_timer(sk, ICSK_TIME_RETRANS);
>
> I have never seen this report before.
Do you use Fast Open? I am wondering if its a bug when a TFO server
closes the socket before the handshake finishes...
Either way, it's pretty safe to just stop TLP if write queue is empty
for any unexpected reason.
>
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf: test_bpf: add init_net to dev for flow_dissector
From: Daniel Borkmann @ 2018-09-27 19:13 UTC (permalink / raw)
To: Willem de Bruijn, Eric Dumazet
Cc: songliubraving, Network Development, Kernel Team,
Willem de Bruijn, Petar Penkov
In-Reply-To: <CAF=yD-K4wxZoNLDwba_-AEqWkN=5wz0XvW0eRrjZnDPqchhCuQ@mail.gmail.com>
On 09/27/2018 06:42 PM, Willem de Bruijn wrote:
> On Thu, Sep 27, 2018 at 12:40 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On 09/27/2018 09:34 AM, Song Liu wrote:
>>> Latest changes in __skb_flow_dissect() assume skb->dev has valid nd_net.
>>> However, this is not true for test_bpf. As a result, test_bpf.ko crashes
>>> the system with the following stack trace:
>>>
>>> This patch fixes tes_bpf by using init_net in the dummy dev.
>>>
>>> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
>>> Reported-by: Eric Dumazet <edumazet@google.com>
>>> Cc: Willem de Bruijn <willemb@google.com>
>>> Cc: Petar Penkov <ppenkov@google.com>
>>> Signed-off-by: Song Liu <songliubraving@fb.com>
>>> ---
>>
>> Reviewed-by: Eric Dumazet <edumazet@google.com>
>
> Acked-by: Willem de Bruijn <willemb@google.com>
>
> Thanks!
Applied to bpf-next, thanks everyone!
^ permalink raw reply
* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Eric Biggers @ 2018-09-28 1:17 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-kernel, netdev, linux-crypto, davem, gregkh
In-Reply-To: <20180927213537.GA27576@zx2c4.com>
On Thu, Sep 27, 2018 at 11:35:39PM +0200, Jason A. Donenfeld wrote:
> Hi Eric,
>
> On Thu, Sep 27, 2018 at 8:29 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > Why is Herbert Xu's existing crypto tree being circumvented, especially for
> > future patches (the initial merge isn't quite as important as that's a one-time
> > event)? I like being able to check out cryptodev to test upcoming crypto
> > patches. And currently, changes to APIs, algorithms, tests, and implementations
> > all go through cryptodev, which is convenient for crypto developers.
> >
> > Apparently, you're proposing that someone adding a new algorithm will now have
> > to submit the API portion to one maintainer (Herbert Xu) and the implementation
> > portion to another maintainer (you), and they'll go through separate git trees.
> > That's inconvenient for developers, and it seems that in practice you and
> > Herbert will be stepping on each other's toes a lot.
> >
> > Can you please reach some kind of sane agreement with Herbert so that the
> > development process isn't fractured into two? Perhaps you could review patches,
> > but Herbert could still apply them?
>
> I think you're overthinking it a bit. Zinc will have a few software
> implementations of primitives that are useful in cases where it's nice to call
> the primitive directly. Think: various usages of sha2, siphash, the wireguard
> suite (what this patchset includes), other things in lib/, etc. In so much as
> this winds up duplicating things within the crypto API, I'll work with Herbert
> to build one on top of the other -- as I've done in the two commits in this
> series. But beyond that, think of the two initiatives as orthogonal. I'm
> working on curating a few primitives that are maximally useful throughout
> the kernel for various uses, and doing so in a way that I think brings
> about a certain quality. Meanwhile the crypto API is amassing a huge
> collection of primitives for some things, and that will continue to exist,
> and Herbert will continue to maintain that. I expect for the crossover
> to be fairly isolated and manageable, without too much foreseeable tree-
> conflicts and such. Therefore, Samuel Neves and I plan to maintain the
> codebase we've spent quite some time writing, and maintain our own tree for
> it, which we'll be submitting through Greg. In other words, this is not
> a matter of "circumvention" or "stepping on toes", but rather separate
> efforts. I'm quite certain to the extent they overlap we'll be able to work
> out fairly easily.
>
> Either way, I'll take your suggestion and reach out to Herbert, since at
> least a discussion between the two of us sounds like it could be productive.
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...
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. And
yes, you and Herbert will step on each others' toes and duplicate stuff, as the
efforts are *not* separate, as you've even argued yourself.
Please reach out to Herbert to find a sane solution, ideally one that involves
having a single git tree for crypto development and allows people to continue
crypto development without choosing "sides".
>
> > I'm also wondering about the criteria for making additions and changes to
> > "Zinc". You mentioned before that one of the "advantages" of Zinc is that it
> > doesn't include "cipher modes from 90s cryptographers" -- what does that mean
> > exactly? You've also indicated before that you don't want people modifying the
> > Poly1305 implementations as they are too error-prone. Useful contributions
> > could be blocked or discouraged in the future. Can you please elaborate on
> > your criteria for contributions to Zinc?
> >
> > Also, will you allow algorithms that aren't up to modern security standards but
> > are needed for compatibility reasons, e.g. MD5, SHA-1, and DES? There are
> > existing standards, APIs, and data formats that use these "legacy" algorithms;
> > so implementations of them are often still needed, whether we like it or not.
> >
> > And does it matter who designed the algorithms, e.g. do algorithms from Daniel
> > Bernstein get effectively a free pass, while algorithms from certain countries,
> > governments, or organizations are not allowed? E.g. wireless driver developers
> > may need the SM4 block cipher (which is now supported by the crypto API) as it's
> > specified in a Chinese wireless standard. Will you allow SM4 in Zinc? Or will
> > people have to submit some algorithms to Herbert and some to you due to
> > disagreements about what algorithms should be included?
>
> Similarly here, I think you're over-politicizing everything. Stable address
> generation for IPv6 uses SHA1 -- see net/ipv6/addrconf.c:3203 -- do you think
> that this should use, say, the SM3 chinese hash function instead? No, of
> course not, for a variety of interesting reasons. Rather, it should use some
> simple hash function that's fast in software that we have available in Zinc.
> On the other hand, it seems like parts of the kernel that have pretty high-
> levels of cipher agility -- such as dmcrypt, ipsec, wifi apparently, and
> so on -- will continue to use dynamic-dispatch system like the crypto API,
> since that's what it was made to do and is effective at doing. And so, your
> example of SM4 seems to fit perfectly into what the crypto API is well-suited
> for, and it would fit naturally in there.
>
> In other words, the "political criteria" for what we add to lib/zinc/ will
> mostly be the same as for the rest of lib/: are there things using it that
> benefit from it being there in a direct and obvious way, and does the
> implementation meet certain quality standards.
>
So, crypto implementations and algorithms will go to different maintainers,
source locations, and git trees based purely on whether the current users need
"cipher agility"? Note that usage can change over time; a user that requires a
single cipher could later need multiple, and vice versa.
What if the portion of a wireless driver that needs SM4 doesn't need any other
cipher in the same place, so static dispatch would suffice? Would SM4 be
allowed in Zinc then?
> > to change them yourself, e.g. when you added the part that converts the
> > accumulator from base 26 to base 32. I worry there may be double standards
> > here
>
> We do actually appreciate your concern here. However, there's a lot more that
> went into that short patch than meets the eye:
>
> - 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...
> - 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?
> - 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 been some considerable computing power sunk into fuzzing it.
>
> There's no doubt about it, we've done our due-diligence here.
Apparently not, given that it's broken on big endian ARM.
> fact the kind of efforts we require of submissions. You could fault us for
> not detailing this in "the commit message" -- except as this is still a
> patch series, we're putting improvements into the 00/XX change log, instead
> of adding fixes and additions on top of the series.
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. Of course, having bugs in code which you insist was proven correct
+ fuzzed doesn't exactly inspire trust.
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.
We need "Zinc" to be Linux's crypto library, not "Jason's crypto library".
- Eric
^ permalink raw reply
* RE: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Chris Preimesberger @ 2018-09-27 18:56 UTC (permalink / raw)
To: 'Andrew Lunn', Eran Ben Elisha
Cc: Neil Horman, linville@tuxdriver.com, netdev@vger.kernel.org
In-Reply-To: <20180927163805.GI12979@lunn.ch>
I greatly appreciate everyone's work on this. Thank you to all.
I've had Mellanox support case # 00508027 open for this issue,
and just now requested an updated driver from them to resolve,
explaining that really smart ethtool developers figured out this
was due to the Mellanox driver not reporting thresholds to ethtool.
I intend to post back here for posterity if/when I get an updated
driver that fixes the issue.
Thanks again!!
Chris Preimesberger | Test & Validation Engineer
Transition Networks, Inc.
chrisp@transition.com
direct: +1.952.996.1509 | fax: +1.952.941.2322 | www.transition.com
-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Thursday, September 27, 2018 11:38 AM
To: Chris Preimesberger
Cc: Eran Ben Elisha; Neil Horman; linville@tuxdriver.com; netdev@vger.kernel.org
Subject: Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
On Thu, Sep 27, 2018 at 04:08:24PM +0000, Chris Preimesberger wrote:
> Please correct me if I'm wrong, but...
> It looks like Eran's proposed fix would remove all warning and alarm
> indications from ethtool's output. It's worth mentioning that for me,
> the following fields always reported correctly as Off while no alarm
> condition was present and On while alarm condition(s) were present
> *per the QSFP's true/programmed threshold values* *not per the
> incorrectly reported threshold values*
These alarm values are in the first page. So the information the driver returns does contain this information. What is missing is the thresholds, which are not provided by the driver.
But there is a comment in the code:
/*
* There is no clear identifier to signify the existence of
* optical diagnostics similar to SFF-8472. So checking existence
* of page 3, will provide the gurantee for existence of alarms
* and thresholds
* If pagging support exists, then supports_alarms is marked as 1
*/
These alarm values are optional. The spec says so. So in order to decide if they are implemented, ethtool looks to see if the thresholds are available. If there are thresholds, it makes sense the alarms are implemented.
Unfortunately, the driver never returns the thresholds. So ethtool has no real choice and won't display the alarms since it cannot determine if they are valid.
In order to get alarms, the driver needs to be extended to return all the pages.
Andrew
^ 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