* [PATCH bpf-next v2] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-06-26 8:33 UTC (permalink / raw)
To: netdev
Cc: ast, daniel, kafai, songliubraving, yhs, andrii.nakryiko,
magnus.karlsson
When an AF_XDP application received X packets, it does not mean X
frames can be stuffed into the producer ring. To make it easier for
AF_XDP applications this API allows them to check how many frames can
be added into the ring.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
v1 -> v2
- Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
- Add caching so it will only touch global state when needed
tools/lib/bpf/xsk.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 82ea71a0f3ec..6acb81102346 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -76,11 +76,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
return &descs[idx & rx->mask];
}
-static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
+static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32 nb)
{
__u32 free_entries = r->cached_cons - r->cached_prod;
- if (free_entries >= nb)
+ if (free_entries >= nb && nb != 0)
return free_entries;
/* Refresh the local tail pointer.
@@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
size_t nb, __u32 *idx)
{
- if (xsk_prod_nb_free(prod, nb) < nb)
+ if (xsk_prod__nb_free(prod, nb) < nb)
return 0;
*idx = prod->cached_prod;
--
2.20.1
^ permalink raw reply related
* XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
From: Jesper Dangaard Brouer @ 2019-06-26 8:38 UTC (permalink / raw)
To: Machulsky, Zorik
Cc: Jubran, Samih, davem@davemloft.net, netdev@vger.kernel.org,
Woodhouse, David, Matushevsky, Alexander, Bshara, Saeed,
Wilson, Matt, Liguori, Anthony, Bshara, Nafea, Tzalik, Guy,
Belgazal, Netanel, Saidi, Ali, Herrenschmidt, Benjamin,
Kiyanovski, Arthur, Daniel Borkmann, brouer,
Toke Høiland-Jørgensen, Ilias Apalodimas,
Alexei Starovoitov, Jakub Kicinski, xdp-newbies@vger.kernel.org
In-Reply-To: <A658E65E-93D2-4F10-823D-CC25B081C1B7@amazon.com>
On Tue, 25 Jun 2019 03:19:22 +0000
"Machulsky, Zorik" <zorik@amazon.com> wrote:
> On 6/23/19, 7:21 AM, "Jesper Dangaard Brouer" <brouer@redhat.com> wrote:
>
> On Sun, 23 Jun 2019 10:06:49 +0300 <sameehj@amazon.com> wrote:
>
> > This commit implements the basic functionality of drop/pass logic in the
> > ena driver.
>
> Usually we require a driver to implement all the XDP return codes,
> before we accept it. But as Daniel and I discussed with Zorik during
> NetConf[1], we are going to make an exception and accept the driver
> if you also implement XDP_TX.
>
> As we trust that Zorik/Amazon will follow and implement XDP_REDIRECT
> later, given he/you wants AF_XDP support which requires XDP_REDIRECT.
>
> Jesper, thanks for your comments and very helpful discussion during
> NetConf! That's the plan, as we agreed. From our side I would like to
> reiterate again the importance of multi-buffer support by xdp frame.
> We would really prefer not to see our MTU shrinking because of xdp
> support.
Okay we really need to make a serious attempt to find a way to support
multi-buffer packets with XDP. With the important criteria of not
hurting performance of the single-buffer per packet design.
I've created a design document[2], that I will update based on our
discussions: [2] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
The use-case that really convinced me was Eric's packet header-split.
Lets refresh: Why XDP don't have multi-buffer support:
XDP is designed for maximum performance, which is why certain driver-level
use-cases were not supported, like multi-buffer packets (like jumbo-frames).
As it e.g. complicated the driver RX-loop and memory model handling.
The single buffer per packet design, is also tied into eBPF Direct-Access
(DA) to packet data, which can only be allowed if the packet memory is in
contiguous memory. This DA feature is essential for XDP performance.
One way forward is to define that XDP only get access to the first
packet buffer, and it cannot see subsequent buffers. For XDP_TX and
XDP_REDIRECT to work then XDP still need to carry pointers (plus
len+offset) to the other buffers, which is 16 bytes per extra buffer.
> [1] http://vger.kernel.org/netconf2019.html
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net-next] net: ethtool: Allow parsing ETHER_FLOW types when using flow_rule
From: Maxime Chevallier @ 2019-06-26 8:44 UTC (permalink / raw)
To: davem, Pablo Neira Ayuso, Florian Fainelli, Jiri Pirko,
Jakub Kicinski
Cc: Maxime Chevallier, netdev, linux-kernel, Antoine Tenart,
thomas.petazzoni
When parsing an ethtool_rx_flow_spec, users can specify an ethernet flow
which could contain matches based on the ethernet header, such as the
MAC address, the VLAN tag or the ethertype.
Only the ethtype field is specific to the ether flow, the MAC and vlan
fields are processed using the special FLOW_EXT and FLOW_MAC_EXT flags.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
net/core/ethtool.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4d1011b2e24f..01ceba556341 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2883,6 +2883,18 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
match->mask.basic.n_proto = htons(0xffff);
switch (fs->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS)) {
+ case ETHER_FLOW: {
+ const struct ethhdr *ether_spec, *ether_m_spec;
+
+ ether_spec = &fs->h_u.ether_spec;
+ ether_m_spec = &fs->m_u.ether_spec;
+
+ if (ether_m_spec->h_proto) {
+ match->key.basic.n_proto = ether_spec->h_proto;
+ match->mask.basic.n_proto = ether_m_spec->h_proto;
+ }
+ }
+ break;
case TCP_V4_FLOW:
case UDP_V4_FLOW: {
const struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Vladimir Oltean @ 2019-06-26 8:46 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <20190626074158.odyrgzie7sv4ovtn@shell.armlinux.org.uk>
On Wed, 26 Jun 2019 at 10:42, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Jun 26, 2019 at 02:10:27AM +0300, Vladimir Oltean wrote:
> > On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > > > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > > > Hi Russell,
> > > > > >
> > > > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > > > This should be removed - state->link is not for use in mac_config.
> > > > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > > > >
> > > > > >
> > > > > > What do you mean exactly that state->link is not for use, is that true in
> > > > > > general?
> > > > >
> > > > > Yes. mac_config() should not touch it; it is not always in a defined
> > > > > state. For example, if you set modes via ethtool (the
> > > > > ethtool_ksettings_set API) then state->link will probably contain
> > > > > zero irrespective of the true link state.
> > > > >
> > > >
> > > > Experimentally, state->link is zero at the same time as state->speed
> > > > is -1, so just ignoring !state->link made sense. This is not in-band
> > > > AN. What is your suggestion? Should I proceed to try and configure the
> > > > MAC for SPEED_UNKNOWN?
> > >
> > > What would you have done with a PHY when the link is down, what speed
> > > would you have configured in the phylib adjust_link callback? phylib
> > > also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
> > >
> >
> > With phylib, I'd make the driver ignore the speed and do nothing.
> > With phylink, I'd make the core not call mac_config.
> > But what happened is I saw phylink call mac_config anyway, said
> > 'weird' and proceeded to ignore it as I would have for phylib.
> > I'm just not understanding your position - it seems like you're
> > implying there's a bug in phylink and the function call with
> > MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken
> > place, which is what I wanted to confirm.
>
> It is not a bug. It is a request to configure the MAC, and what it's
> saying is "we don't know what speed and/or duplex".
>
> Take for instance when the network adapter is brought up initially.
> The link is most likely down, but we should configure the initial MAC
> operating parameters (such as the PHY interface). Phylink makes a
> mac_config() call with the speed and duplex set to UNKNOWN.
>
> Using your theory, we shouldn't be making that call. In which case,
> MAC drivers aren't going to initially configure their interface
> settings.
>
> _That_ would be a bug.
>
So you're saying that:
- state->link should not be checked, because it is not guaranteed to be valid
- state->speed, state->duplex, state->pause *should* be checked,
because it is not guaranteed to be valid
Is state->interface always valid?
I don't think I follow the pattern here. Or shouldn't I check speed,
duplex and pause either, and try to pass the MAC UNKNOWN values,
inevitably failing at some point? Do Marvell MACs have an UNKNOWN
setting?
> > It's unlikely that it would switch between SGMII and USXGMII
> > > dynamically, as USXGMII supports speeds from 10G down to 10M.
> > >
> > > Where interface mode switching tends to be used is with modes such
> > > as 10GBASE-R, which doesn't support anything except 10G. In order
> > > for the PHY to operate at slower speeds, it has a few options:
> > >
> > > 1) perform rate adaption.
> > > 2) dynamically switch interface type to an interface type that
> > > supports the desired speed.
> > > 3) just not support slower speeds.
> > >
> >
> > So am I reading this correctly - it kind of makes sense for gigabit
> > MAC drivers to not check for the MII interface changing protocol?
>
> Again, that's incorrect in the general case. Gigabit includes SGMII
> and 802.3z PHY protocols which need to be switched between for SFPs.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
Thanks,
-Vladimir
^ permalink raw reply
* Re: [PATCH net-next] net: ethtool: Allow parsing ETHER_FLOW types when using flow_rule
From: Pablo Neira Ayuso @ 2019-06-26 8:58 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Florian Fainelli, Jiri Pirko, Jakub Kicinski, netdev,
linux-kernel, Antoine Tenart, thomas.petazzoni
In-Reply-To: <20190626084403.17749-1-maxime.chevallier@bootlin.com>
On Wed, Jun 26, 2019 at 10:44:03AM +0200, Maxime Chevallier wrote:
> When parsing an ethtool_rx_flow_spec, users can specify an ethernet flow
> which could contain matches based on the ethernet header, such as the
> MAC address, the VLAN tag or the ethertype.
>
> Only the ethtype field is specific to the ether flow, the MAC and vlan
> fields are processed using the special FLOW_EXT and FLOW_MAC_EXT flags.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> net/core/ethtool.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 4d1011b2e24f..01ceba556341 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -2883,6 +2883,18 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
> match->mask.basic.n_proto = htons(0xffff);
>
> switch (fs->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS)) {
> + case ETHER_FLOW: {
> + const struct ethhdr *ether_spec, *ether_m_spec;
> +
> + ether_spec = &fs->h_u.ether_spec;
> + ether_m_spec = &fs->m_u.ether_spec;
> +
> + if (ether_m_spec->h_proto) {
> + match->key.basic.n_proto = ether_spec->h_proto;
> + match->mask.basic.n_proto = ether_m_spec->h_proto;
> + }
I see some drivers in the tree also interpret the h_source and h_dest
fields?
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/sfc/falcon/ethtool.c#L1182
Probably good to address this in this patch too?
Thanks.
^ permalink raw reply
* RE: [PATCH net-next 01/16] qlge: Remove irq_cnt
From: Manish Chopra @ 2019-06-26 8:59 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-1-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [PATCH net-next 01/16] qlge: Remove irq_cnt
>
> qlge uses an irq enable/disable refcounting scheme that is:
> * poorly implemented
> Uses a spin_lock to protect accesses to the irq_cnt atomic variable
> * buggy
> Breaks when there is not a 1:1 sequence of irq - napi_poll, such as
> when using SO_BUSY_POLL.
> * unnecessary
> The purpose or irq_cnt is to reduce irq control writes when
> multiple work items result from one irq: the irq is re-enabled
> after all work is done.
> Analysis of the irq handler shows that there is only one case where
> there might be two workers scheduled at once, and those have
> separate irq masking bits.
I believe you are talking about here for asic error recovery worker and MPI worker.
Which separate IRQ masking bits are you referring here ?
> static int ql_validate_flash(struct ql_adapter *qdev, u32 size, const char *str)
> @@ -2500,21 +2451,22 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
> u32 var;
> int work_done = 0;
>
> - spin_lock(&qdev->hw_lock);
> - if (atomic_read(&qdev->intr_context[0].irq_cnt)) {
> - netif_printk(qdev, intr, KERN_DEBUG, qdev->ndev,
> - "Shared Interrupt, Not ours!\n");
> - spin_unlock(&qdev->hw_lock);
> - return IRQ_NONE;
> - }
> - spin_unlock(&qdev->hw_lock);
> + /* Experience shows that when using INTx interrupts, the device does
> + * not always auto-mask the interrupt.
> + * When using MSI mode, the interrupt must be explicitly disabled
> + * (even though it is auto-masked), otherwise a later command to
> + * enable it is not effective.
> + */
> + if (!test_bit(QL_MSIX_ENABLED, &qdev->flags))
> + ql_disable_completion_interrupt(qdev, 0);
Current code is disabling completion interrupt in case of MSI-X zeroth vector.
This change will break that behavior. Shouldn't zeroth vector in case of MSI-X also disable completion interrupt ?
If not, please explain why ?
>
> - var = ql_disable_completion_interrupt(qdev, intr_context->intr);
> + var = ql_read32(qdev, STS);
>
> /*
> * Check for fatal error.
> */
> if (var & STS_FE) {
> + ql_disable_completion_interrupt(qdev, 0);
Why need to do it again here ? if before this it can disable completion interrupt for INT-X case and MSI-X zeroth vector case ?
> ql_queue_asic_error(qdev);
> netdev_err(qdev->ndev, "Got fatal error, STS = %x.\n", var);
> var = ql_read32(qdev, ERR_STS);
> @@ -2534,7 +2486,6 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
> */
> netif_err(qdev, intr, qdev->ndev,
> "Got MPI processor interrupt.\n");
> - ql_disable_completion_interrupt(qdev, intr_context->intr);
Why disable interrupt is not required here ? While in case of Fatal error case above ql_disable_completion_interrupt() is being called ?
Also, in case of MSI-X zeroth vector it will not disable completion interrupt but at the end, it will end of qlge_isr() enabling completion interrupt.
Seems like disabling and enabling might not be in sync in case of MSI-X zeroth vector.
^ permalink raw reply
* Re: [PATCH nf-next v2 1/2] netfilter: nft_meta: add NFT_META_BRI_PVID support
From: Pablo Neira Ayuso @ 2019-06-26 9:01 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1560993460-25569-1-git-send-email-wenxu@ucloud.cn>
On Thu, Jun 20, 2019 at 09:17:39AM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> nft add table bridge firewall
> nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
> nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
>
> As above set the bridge port with pvid, the received packet don't contain
> the vlan tag which means the packet should belong to vlan 200 through pvid.
> With this pacth user can get the pvid of bridge ports.
>
> So add the following rule for as the first rule in the chain of zones.
>
> nft add rule bridge firewall zones counter meta brvlan set meta brpvid
Applied, thanks.
^ permalink raw reply
* Re: [PATCH nf-next v2 2/2] netfilter: nft_meta: Add NFT_META_BRI_VLAN support
From: Pablo Neira Ayuso @ 2019-06-26 9:01 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1560993460-25569-2-git-send-email-wenxu@ucloud.cn>
On Thu, Jun 20, 2019 at 09:17:40AM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> nft add table bridge firewall
> nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
> nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
>
> As above set the bridge port with pvid, the received packet don't contain
> the vlan tag which means the packet should belong to vlan 200 through pvid.
> With this pacth user can set the pvid in the prerouting hook before set zone
> id and conntrack.
>
> So add the following rule for as the first rule in the chain of zones.
>
> nft add rule bridge firewall zones counter meta brvlan set meta brpvid
Also applied, thanks.
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Russell King - ARM Linux admin @ 2019-06-26 9:04 UTC (permalink / raw)
To: Vladimir Oltean
Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <CA+h21hpkjHD07-o7W-5sUf+FqEeks17_W6VUROSDzdGokFvNWQ@mail.gmail.com>
On Wed, Jun 26, 2019 at 11:46:15AM +0300, Vladimir Oltean wrote:
> On Wed, 26 Jun 2019 at 10:42, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Jun 26, 2019 at 02:10:27AM +0300, Vladimir Oltean wrote:
> > > On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > > > > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > > > > <linux@armlinux.org.uk> wrote:
> > > > > >
> > > > > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > > > > Hi Russell,
> > > > > > >
> > > > > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > > > > This should be removed - state->link is not for use in mac_config.
> > > > > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > > > > >
> > > > > > >
> > > > > > > What do you mean exactly that state->link is not for use, is that true in
> > > > > > > general?
> > > > > >
> > > > > > Yes. mac_config() should not touch it; it is not always in a defined
> > > > > > state. For example, if you set modes via ethtool (the
> > > > > > ethtool_ksettings_set API) then state->link will probably contain
> > > > > > zero irrespective of the true link state.
> > > > > >
> > > > >
> > > > > Experimentally, state->link is zero at the same time as state->speed
> > > > > is -1, so just ignoring !state->link made sense. This is not in-band
> > > > > AN. What is your suggestion? Should I proceed to try and configure the
> > > > > MAC for SPEED_UNKNOWN?
> > > >
> > > > What would you have done with a PHY when the link is down, what speed
> > > > would you have configured in the phylib adjust_link callback? phylib
> > > > also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
> > > >
> > >
> > > With phylib, I'd make the driver ignore the speed and do nothing.
> > > With phylink, I'd make the core not call mac_config.
> > > But what happened is I saw phylink call mac_config anyway, said
> > > 'weird' and proceeded to ignore it as I would have for phylib.
> > > I'm just not understanding your position - it seems like you're
> > > implying there's a bug in phylink and the function call with
> > > MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken
> > > place, which is what I wanted to confirm.
> >
> > It is not a bug. It is a request to configure the MAC, and what it's
> > saying is "we don't know what speed and/or duplex".
> >
> > Take for instance when the network adapter is brought up initially.
> > The link is most likely down, but we should configure the initial MAC
> > operating parameters (such as the PHY interface). Phylink makes a
> > mac_config() call with the speed and duplex set to UNKNOWN.
> >
> > Using your theory, we shouldn't be making that call. In which case,
> > MAC drivers aren't going to initially configure their interface
> > settings.
> >
> > _That_ would be a bug.
> >
>
> So you're saying that:
> - state->link should not be checked, because it is not guaranteed to be valid
state->link is undefined.
> - state->speed, state->duplex, state->pause *should* be checked,
These will always be valid for FIXED and PHY modes, but _may_ be
UNKNOWN, meaning phylink does not have any information about what
the speed should be.
speed and duplex are not defined for inband modes, since the purpose
of inband modes is to communicate this information through... inband
information, which the MAC driver already has access to. pause is
a different matter because it is present in some inband modes but
not others.
Which fields may be examined are now documented in the phylink
documentation in mainline kernels.
> Is state->interface always valid?
Yes.
> I don't think I follow the pattern here. Or shouldn't I check speed,
> duplex and pause either, and try to pass the MAC UNKNOWN values,
> inevitably failing at some point? Do Marvell MACs have an UNKNOWN
> setting?
Why do you think that just because state->speed is SPEED_UNKNOWN you
have to dream up some weird "unknown" value for the MAC? Default it
to something sensible, just like you would do if phylib reports
SPEED_UNKNOWN during link-down. I really don't get what the problem
is here.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [bpf-next v2 08/10] bpf: Implement bpf_prog_test_run for perf event programs
From: Krzesimir Nowak @ 2019-06-26 9:10 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, Alban Crequy, Iago López Galeiras,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, linux-kernel, bpf
In-Reply-To: <20190625201220.GC10487@mini-arch>
On Tue, Jun 25, 2019 at 10:12 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/25, Krzesimir Nowak wrote:
> > As an input, test run for perf event program takes struct
> > bpf_perf_event_data as ctx_in and struct bpf_perf_event_value as
> > data_in. For an output, it basically ignores ctx_out and data_out.
> >
> > The implementation sets an instance of struct bpf_perf_event_data_kern
> > in such a way that the BPF program reading data from context will
> > receive what we passed to the bpf prog test run in ctx_in. Also BPF
> > program can call bpf_perf_prog_read_value to receive what was passed
> > in data_in.
> >
> > Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
> > ---
> > kernel/trace/bpf_trace.c | 107 ++++++++++++++++++
> > .../bpf/verifier/perf_event_sample_period.c | 8 ++
> > 2 files changed, 115 insertions(+)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index c102c240bb0b..2fa49ea8a475 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -16,6 +16,8 @@
> >
> > #include <asm/tlb.h>
> >
> > +#include <trace/events/bpf_test_run.h>
> > +
> > #include "trace_probe.h"
> > #include "trace.h"
> >
> > @@ -1160,7 +1162,112 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
> > .convert_ctx_access = pe_prog_convert_ctx_access,
> > };
> >
> > +static int pe_prog_test_run(struct bpf_prog *prog,
> > + const union bpf_attr *kattr,
> > + union bpf_attr __user *uattr)
> > +{
> > + void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
> > + void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
> > + u32 data_size_in = kattr->test.data_size_in;
> > + u32 ctx_size_in = kattr->test.ctx_size_in;
> > + u32 repeat = kattr->test.repeat;
> > + u32 retval = 0, duration = 0;
> > + int err = -EINVAL;
> > + u64 time_start, time_spent = 0;
> > + int i;
> > + struct perf_sample_data sample_data = {0, };
> > + struct perf_event event = {0, };
> > + struct bpf_perf_event_data_kern real_ctx = {0, };
> > + struct bpf_perf_event_data fake_ctx = {0, };
> > + struct bpf_perf_event_value value = {0, };
> > +
> > + if (ctx_size_in != sizeof(fake_ctx))
> > + goto out;
> > + if (data_size_in != sizeof(value))
> > + goto out;
> > +
> > + if (copy_from_user(&fake_ctx, ctx_in, ctx_size_in)) {
> > + err = -EFAULT;
> > + goto out;
> > + }
> Move this to net/bpf/test_run.c? I have a bpf_ctx_init helper to deal
> with ctx input, might save you some code above wrt ctx size/etc.
My impression about net/bpf/test_run.c was that it was a collection of
helpers for test runs of the network-related BPF programs, because
they are so similar to each other. So kernel/trace/bpf_trace.c looked
like an obvious place for the test_run implementation since other perf
trace BPF stuff was already there.
And about bpf_ctx_init - looks useful as it seems to me that it
handles the scenario where the size of the ctx struct grows, but still
allows passing older version of the struct (thus smaller) from
userspace for compatibility. Maybe that checking and copying part of
the function could be moved into some non-static helper function, so I
could use it and still skip the need for allocating memory for the
context?
>
> > + if (copy_from_user(&value, data_in, data_size_in)) {
> > + err = -EFAULT;
> > + goto out;
> > + }
> > +
> > + real_ctx.regs = &fake_ctx.regs;
> > + real_ctx.data = &sample_data;
> > + real_ctx.event = &event;
> > + perf_sample_data_init(&sample_data, fake_ctx.addr,
> > + fake_ctx.sample_period);
> > + event.cpu = smp_processor_id();
> > + event.oncpu = -1;
> > + event.state = PERF_EVENT_STATE_OFF;
> > + local64_set(&event.count, value.counter);
> > + event.total_time_enabled = value.enabled;
> > + event.total_time_running = value.running;
> > + /* make self as a leader - it is used only for checking the
> > + * state field
> > + */
> > + event.group_leader = &event;
> > +
> > + /* slightly changed copy pasta from bpf_test_run() in
> > + * net/bpf/test_run.c
> > + */
> > + if (!repeat)
> > + repeat = 1;
> > +
> > + rcu_read_lock();
> > + preempt_disable();
> > + time_start = ktime_get_ns();
> > + for (i = 0; i < repeat; i++) {
> Any reason for not using bpf_test_run?
Two, mostly. One was that it is a static function and my code was
elsewhere. Second was that it does some cgroup storage setup and I'm
not sure if the perf event BPF program needs that.
>
> > + retval = BPF_PROG_RUN(prog, &real_ctx);
> > +
> > + if (signal_pending(current)) {
> > + err = -EINTR;
> > + preempt_enable();
> > + rcu_read_unlock();
> > + goto out;
> > + }
> > +
> > + if (need_resched()) {
> > + time_spent += ktime_get_ns() - time_start;
> > + preempt_enable();
> > + rcu_read_unlock();
> > +
> > + cond_resched();
> > +
> > + rcu_read_lock();
> > + preempt_disable();
> > + time_start = ktime_get_ns();
> > + }
> > + }
> > + time_spent += ktime_get_ns() - time_start;
> > + preempt_enable();
> > + rcu_read_unlock();
> > +
> > + do_div(time_spent, repeat);
> > + duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> > + /* end of slightly changed copy pasta from bpf_test_run() in
> > + * net/bpf/test_run.c
> > + */
> > +
> > + if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval))) {
> > + err = -EFAULT;
> > + goto out;
> > + }
> > + if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration))) {
> > + err = -EFAULT;
> > + goto out;
> > + }
> Can BPF program modify fake_ctx? Do we need/want to copy it back?
Reading the pe_prog_is_valid_access function tells me that it's not
possible - the only type of valid access is read. So maybe I should be
stricter about the requirements for the data_out and ctx_out sizes
(should be zero or return -EINVAL).
>
> > + err = 0;
> > +out:
> > + trace_bpf_test_finish(&err);
> > + return err;
> > +}
> > +
> > const struct bpf_prog_ops perf_event_prog_ops = {
> > + .test_run = pe_prog_test_run,
> > };
> >
> > static DEFINE_MUTEX(bpf_event_mutex);
> > diff --git a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > index 471c1a5950d8..16e9e5824d14 100644
> > --- a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > +++ b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> This should probably go in another patch.
Yeah, I was wondering about it. These changes are here to avoid
breaking the tests, since perf event program can actually be run now
and the test_run for perf event required certain sizes for ctx and
data.
So, I will either move them to a separate patch or rework the test_run
for perf event to accept the size between 0 and sizeof(struct
something), so the changes in tests maybe will not be necessary.
>
> > @@ -13,6 +13,8 @@
> > },
> > .result = ACCEPT,
> > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > + .data_len = sizeof(struct bpf_perf_event_value),
> > },
> > {
> > "check bpf_perf_event_data->sample_period half load permitted",
> > @@ -29,6 +31,8 @@
> > },
> > .result = ACCEPT,
> > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > + .data_len = sizeof(struct bpf_perf_event_value),
> > },
> > {
> > "check bpf_perf_event_data->sample_period word load permitted",
> > @@ -45,6 +49,8 @@
> > },
> > .result = ACCEPT,
> > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > + .data_len = sizeof(struct bpf_perf_event_value),
> > },
> > {
> > "check bpf_perf_event_data->sample_period dword load permitted",
> > @@ -56,4 +62,6 @@
> > },
> > .result = ACCEPT,
> > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > + .data_len = sizeof(struct bpf_perf_event_value),
> > },
> > --
> > 2.20.1
> >
--
Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
Registergericht/Court of registration: Amtsgericht Charlottenburg
Registernummer/Registration number: HRB 171414 B
Ust-ID-Nummer/VAT ID number: DE302207000
^ permalink raw reply
* RE: [PATCH net-next 02/16] qlge: Remove page_chunk.last_flag
From: Manish Chopra @ 2019-06-26 9:12 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-2-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [PATCH net-next 02/16] qlge: Remove page_chunk.last_flag
>
> As already done in ql_get_curr_lchunk(), this member can be replaced by a
> simple test.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/qlogic/qlge/qlge.h | 1 -
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 13 +++++--------
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h
> b/drivers/net/ethernet/qlogic/qlge/qlge.h
> index 5d9a36deda08..0a156a95e981 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge.h
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
> @@ -1363,7 +1363,6 @@ struct page_chunk {
> char *va; /* virt addr for this chunk */
> u64 map; /* mapping for master */
> unsigned int offset; /* offset for this chunk */
> - unsigned int last_flag; /* flag set for last chunk in page */
> };
>
> struct bq_desc {
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 0bfbe11db795..038a6bfc79c7 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -1077,11 +1077,9 @@ static int ql_get_next_chunk(struct ql_adapter
> *qdev, struct rx_ring *rx_ring,
> rx_ring->pg_chunk.offset += rx_ring->lbq_buf_size;
> if (rx_ring->pg_chunk.offset == ql_lbq_block_size(qdev)) {
> rx_ring->pg_chunk.page = NULL;
> - lbq_desc->p.pg_chunk.last_flag = 1;
> } else {
> rx_ring->pg_chunk.va += rx_ring->lbq_buf_size;
> get_page(rx_ring->pg_chunk.page);
> - lbq_desc->p.pg_chunk.last_flag = 0;
> }
> return 0;
> }
> @@ -2778,6 +2776,8 @@ static int ql_alloc_tx_resources(struct ql_adapter
> *qdev,
>
> static void ql_free_lbq_buffers(struct ql_adapter *qdev, struct rx_ring
> *rx_ring) {
> + unsigned int last_offset = ql_lbq_block_size(qdev) -
> + rx_ring->lbq_buf_size;
> struct bq_desc *lbq_desc;
>
> uint32_t curr_idx, clean_idx;
> @@ -2787,13 +2787,10 @@ static void ql_free_lbq_buffers(struct ql_adapter
> *qdev, struct rx_ring *rx_ring
> while (curr_idx != clean_idx) {
> lbq_desc = &rx_ring->lbq[curr_idx];
>
> - if (lbq_desc->p.pg_chunk.last_flag) {
> - pci_unmap_page(qdev->pdev,
> - lbq_desc->p.pg_chunk.map,
> - ql_lbq_block_size(qdev),
> + if (lbq_desc->p.pg_chunk.offset == last_offset)
> + pci_unmap_page(qdev->pdev, lbq_desc-
> >p.pg_chunk.map,
> + ql_lbq_block_size(qdev),
> PCI_DMA_FROMDEVICE);
> - lbq_desc->p.pg_chunk.last_flag = 0;
> - }
>
> put_page(lbq_desc->p.pg_chunk.page);
> lbq_desc->p.pg_chunk.page = NULL;
> --
> 2.21.0
Acked-by: Manish Chopra <manishc@marvell.com>
^ permalink raw reply
* Re: [PATCH net-next] net: ethtool: Allow parsing ETHER_FLOW types when using flow_rule
From: Maxime Chevallier @ 2019-06-26 9:23 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: davem, Florian Fainelli, Jiri Pirko, Jakub Kicinski, netdev,
linux-kernel, Antoine Tenart, thomas.petazzoni
In-Reply-To: <20190626085846.ax277ojvyp5k3abt@salvia>
Hi Pablo,
On Wed, 26 Jun 2019 10:58:46 +0200
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>On Wed, Jun 26, 2019 at 10:44:03AM +0200, Maxime Chevallier wrote:
>> When parsing an ethtool_rx_flow_spec, users can specify an ethernet flow
>> which could contain matches based on the ethernet header, such as the
>> MAC address, the VLAN tag or the ethertype.
>>
>> Only the ethtype field is specific to the ether flow, the MAC and vlan
>> fields are processed using the special FLOW_EXT and FLOW_MAC_EXT flags.
>>
>> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
>> ---
>> net/core/ethtool.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index 4d1011b2e24f..01ceba556341 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -2883,6 +2883,18 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
>> match->mask.basic.n_proto = htons(0xffff);
>>
>> switch (fs->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS)) {
>> + case ETHER_FLOW: {
>> + const struct ethhdr *ether_spec, *ether_m_spec;
>> +
>> + ether_spec = &fs->h_u.ether_spec;
>> + ether_m_spec = &fs->m_u.ether_spec;
>> +
>> + if (ether_m_spec->h_proto) {
>> + match->key.basic.n_proto = ether_spec->h_proto;
>> + match->mask.basic.n_proto = ether_m_spec->h_proto;
>> + }
>
>I see some drivers in the tree also interpret the h_source and h_dest
>fields?
Ah yes you're right. I assumed these fields were specific to the
FLOW_MAC_EXT flags, but by looking into the ethtool code, it seems we
do need to handle the h_source and h_dest fields.
I'll respin with these fields added.
Thanks for the review,
Maxime
^ permalink raw reply
* RE: [EXT] [PATCH net-next 03/16] qlge: Deduplicate lbq_buf_size
From: Manish Chopra @ 2019-06-26 9:24 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-3-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [EXT] [PATCH net-next 03/16] qlge: Deduplicate lbq_buf_size
>
> External Email
>
> ----------------------------------------------------------------------
> lbq_buf_size is duplicated to every rx_ring structure whereas lbq_buf_order is
> present once in the ql_adapter structure. All rings use the same buf size, keep
> only one copy of it. Also factor out the calculation of lbq_buf_size instead of
> having two copies.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/qlogic/qlge/qlge.h | 2 +-
> drivers/net/ethernet/qlogic/qlge/qlge_dbg.c | 2 +-
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 61 +++++++++-----------
> 3 files changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h
> b/drivers/net/ethernet/qlogic/qlge/qlge.h
> index 0a156a95e981..ba61b4559dd6 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge.h
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
> @@ -1433,7 +1433,6 @@ struct rx_ring {
> /* Large buffer queue elements. */
> u32 lbq_len; /* entry count */
> u32 lbq_size; /* size in bytes of queue */
> - u32 lbq_buf_size;
> void *lbq_base;
> dma_addr_t lbq_base_dma;
> void *lbq_base_indirect;
> @@ -2108,6 +2107,7 @@ struct ql_adapter {
> struct rx_ring rx_ring[MAX_RX_RINGS];
> struct tx_ring tx_ring[MAX_TX_RINGS];
> unsigned int lbq_buf_order;
> + u32 lbq_buf_size;
>
> int rx_csum;
> u32 default_rx_queue;
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> index 31389ab8bdf7..46599d74c6fb 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> @@ -1630,6 +1630,7 @@ void ql_dump_qdev(struct ql_adapter *qdev)
> DUMP_QDEV_FIELD(qdev, "0x%08x", xg_sem_mask);
> DUMP_QDEV_FIELD(qdev, "0x%08x", port_link_up);
> DUMP_QDEV_FIELD(qdev, "0x%08x", port_init);
> + DUMP_QDEV_FIELD(qdev, "%u", lbq_buf_size);
> }
> #endif
>
> @@ -1774,7 +1775,6 @@ void ql_dump_rx_ring(struct rx_ring *rx_ring)
> pr_err("rx_ring->lbq_curr_idx = %d\n", rx_ring->lbq_curr_idx);
> pr_err("rx_ring->lbq_clean_idx = %d\n", rx_ring->lbq_clean_idx);
> pr_err("rx_ring->lbq_free_cnt = %d\n", rx_ring->lbq_free_cnt);
> - pr_err("rx_ring->lbq_buf_size = %d\n", rx_ring->lbq_buf_size);
>
> pr_err("rx_ring->sbq_base = %p\n", rx_ring->sbq_base);
> pr_err("rx_ring->sbq_base_dma = %llx\n", diff --git
> a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 038a6bfc79c7..9df06ad3fb93 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -995,15 +995,14 @@ static struct bq_desc *ql_get_curr_lchunk(struct
> ql_adapter *qdev,
> struct bq_desc *lbq_desc = ql_get_curr_lbuf(rx_ring);
>
> pci_dma_sync_single_for_cpu(qdev->pdev,
> - dma_unmap_addr(lbq_desc,
> mapaddr),
> - rx_ring->lbq_buf_size,
> - PCI_DMA_FROMDEVICE);
> + dma_unmap_addr(lbq_desc, mapaddr),
> + qdev->lbq_buf_size,
> PCI_DMA_FROMDEVICE);
>
> /* If it's the last chunk of our master page then
> * we unmap it.
> */
> - if ((lbq_desc->p.pg_chunk.offset + rx_ring->lbq_buf_size)
> - == ql_lbq_block_size(qdev))
> + if (lbq_desc->p.pg_chunk.offset + qdev->lbq_buf_size ==
> + ql_lbq_block_size(qdev))
> pci_unmap_page(qdev->pdev,
> lbq_desc->p.pg_chunk.map,
> ql_lbq_block_size(qdev),
> @@ -1074,11 +1073,11 @@ static int ql_get_next_chunk(struct ql_adapter
> *qdev, struct rx_ring *rx_ring,
> /* Adjust the master page chunk for next
> * buffer get.
> */
> - rx_ring->pg_chunk.offset += rx_ring->lbq_buf_size;
> + rx_ring->pg_chunk.offset += qdev->lbq_buf_size;
> if (rx_ring->pg_chunk.offset == ql_lbq_block_size(qdev)) {
> rx_ring->pg_chunk.page = NULL;
> } else {
> - rx_ring->pg_chunk.va += rx_ring->lbq_buf_size;
> + rx_ring->pg_chunk.va += qdev->lbq_buf_size;
> get_page(rx_ring->pg_chunk.page);
> }
> return 0;
> @@ -1110,12 +1109,12 @@ static void ql_update_lbq(struct ql_adapter
> *qdev, struct rx_ring *rx_ring)
> lbq_desc->p.pg_chunk.offset;
> dma_unmap_addr_set(lbq_desc, mapaddr, map);
> dma_unmap_len_set(lbq_desc, maplen,
> - rx_ring->lbq_buf_size);
> + qdev->lbq_buf_size);
> *lbq_desc->addr = cpu_to_le64(map);
>
> pci_dma_sync_single_for_device(qdev->pdev, map,
> - rx_ring->lbq_buf_size,
> - PCI_DMA_FROMDEVICE);
> + qdev->lbq_buf_size,
> + PCI_DMA_FROMDEVICE);
> clean_idx++;
> if (clean_idx == rx_ring->lbq_len)
> clean_idx = 0;
> @@ -1880,8 +1879,7 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> }
> do {
> lbq_desc = ql_get_curr_lchunk(qdev, rx_ring);
> - size = (length < rx_ring->lbq_buf_size) ? length :
> - rx_ring->lbq_buf_size;
> + size = min(length, qdev->lbq_buf_size);
>
> netif_printk(qdev, rx_status, KERN_DEBUG, qdev-
> >ndev,
> "Adding page %d to skb for %d bytes.\n",
> @@ -2776,12 +2774,12 @@ static int ql_alloc_tx_resources(struct ql_adapter
> *qdev,
>
> static void ql_free_lbq_buffers(struct ql_adapter *qdev, struct rx_ring
> *rx_ring) {
> - unsigned int last_offset = ql_lbq_block_size(qdev) -
> - rx_ring->lbq_buf_size;
> + unsigned int last_offset;
> struct bq_desc *lbq_desc;
>
> uint32_t curr_idx, clean_idx;
>
> + last_offset = ql_lbq_block_size(qdev) - qdev->lbq_buf_size;
> curr_idx = rx_ring->lbq_curr_idx;
> clean_idx = rx_ring->lbq_clean_idx;
> while (curr_idx != clean_idx) {
> @@ -3149,8 +3147,8 @@ static int ql_start_rx_ring(struct ql_adapter *qdev,
> struct rx_ring *rx_ring)
> } while (page_entries < MAX_DB_PAGES_PER_BQ(rx_ring-
> >lbq_len));
> cqicb->lbq_addr =
> cpu_to_le64(rx_ring->lbq_base_indirect_dma);
> - bq_len = (rx_ring->lbq_buf_size == 65536) ? 0 :
> - (u16) rx_ring->lbq_buf_size;
> + bq_len = (qdev->lbq_buf_size == 65536) ? 0 :
> + (u16)qdev->lbq_buf_size;
> cqicb->lbq_buf_size = cpu_to_le16(bq_len);
> bq_len = (rx_ring->lbq_len == 65536) ? 0 :
> (u16) rx_ring->lbq_len;
> @@ -4048,16 +4046,21 @@ static int qlge_close(struct net_device *ndev)
> return 0;
> }
>
> +static void qlge_set_lb_size(struct ql_adapter *qdev) {
> + if (qdev->ndev->mtu <= 1500)
> + qdev->lbq_buf_size = LARGE_BUFFER_MIN_SIZE;
> + else
> + qdev->lbq_buf_size = LARGE_BUFFER_MAX_SIZE;
> + qdev->lbq_buf_order = get_order(qdev->lbq_buf_size); }
> +
> static int ql_configure_rings(struct ql_adapter *qdev) {
> int i;
> struct rx_ring *rx_ring;
> struct tx_ring *tx_ring;
> int cpu_cnt = min(MAX_CPUS, (int)num_online_cpus());
> - unsigned int lbq_buf_len = (qdev->ndev->mtu > 1500) ?
> - LARGE_BUFFER_MAX_SIZE : LARGE_BUFFER_MIN_SIZE;
> -
> - qdev->lbq_buf_order = get_order(lbq_buf_len);
>
> /* In a perfect world we have one RSS ring for each CPU
> * and each has it's own vector. To do that we ask for @@ -4105,7
> +4108,6 @@ static int ql_configure_rings(struct ql_adapter *qdev)
> rx_ring->lbq_len = NUM_LARGE_BUFFERS;
> rx_ring->lbq_size =
> rx_ring->lbq_len * sizeof(__le64);
> - rx_ring->lbq_buf_size = (u16)lbq_buf_len;
> rx_ring->sbq_len = NUM_SMALL_BUFFERS;
> rx_ring->sbq_size =
> rx_ring->sbq_len * sizeof(__le64); @@ -4121,7
> +4123,6 @@ static int ql_configure_rings(struct ql_adapter *qdev)
> rx_ring->cq_len * sizeof(struct ql_net_rsp_iocb);
> rx_ring->lbq_len = 0;
> rx_ring->lbq_size = 0;
> - rx_ring->lbq_buf_size = 0;
> rx_ring->sbq_len = 0;
> rx_ring->sbq_size = 0;
> rx_ring->sbq_buf_size = 0;
> @@ -4140,6 +4141,7 @@ static int qlge_open(struct net_device *ndev)
> if (err)
> return err;
>
> + qlge_set_lb_size(qdev);
> err = ql_configure_rings(qdev);
> if (err)
> return err;
> @@ -4161,9 +4163,7 @@ static int qlge_open(struct net_device *ndev)
>
> static int ql_change_rx_buffers(struct ql_adapter *qdev) {
> - struct rx_ring *rx_ring;
> - int i, status;
> - u32 lbq_buf_len;
> + int status;
>
> /* Wait for an outstanding reset to complete. */
> if (!test_bit(QL_ADAPTER_UP, &qdev->flags)) { @@ -4186,16 +4186,7
> @@ static int ql_change_rx_buffers(struct ql_adapter *qdev)
> if (status)
> goto error;
>
> - /* Get the new rx buffer size. */
> - lbq_buf_len = (qdev->ndev->mtu > 1500) ?
> - LARGE_BUFFER_MAX_SIZE : LARGE_BUFFER_MIN_SIZE;
> - qdev->lbq_buf_order = get_order(lbq_buf_len);
> -
> - for (i = 0; i < qdev->rss_ring_count; i++) {
> - rx_ring = &qdev->rx_ring[i];
> - /* Set the new size. */
> - rx_ring->lbq_buf_size = lbq_buf_len;
> - }
> + qlge_set_lb_size(qdev);
>
> status = ql_adapter_up(qdev);
> if (status)
> --
> 2.21.0
Not sure if this change is really required, I think fields relevant to rx_ring should be present in the rx_ring structure.
There are various other fields like "lbq_len" and "lbq_size" which would be same for all rx rings but still under the relevant rx_ring structure.
^ permalink raw reply
* Re: [PATCH net-next] can: dev: call netif_carrier_off() in register_candev()
From: Rasmus Villemoes @ 2019-06-26 9:31 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Rasmus Villemoes, linux-can@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CA+FuTSeHhz1kntLyeUfAB4ZbtYjO1=Ornwse-yQbPwo5c-_2=g@mail.gmail.com>
On 24/06/2019 19.26, Willem de Bruijn wrote:
> On Mon, Jun 24, 2019 at 4:34 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>
>> CONFIG_CAN_LEDS is deprecated. When trying to use the generic netdev
>> trigger as suggested, there's a small inconsistency with the link
>> property: The LED is on initially, stays on when the device is brought
>> up, and then turns off (as expected) when the device is brought down.
>>
>> Make sure the LED always reflects the state of the CAN device.
>>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>
> Should this target net?
No, I think this should go through the CAN tree. Perhaps I've
misunderstood when to use the net-next prefix - is that only for things
that should be applied directly to the net-next tree? If so, sorry.
> Regardless of CONFIG_CAN_LEDS deprecation,
> this is already not initialized properly if that CONFIG is disabled
> and a can_led_event call at device probe is a noop.
I'm not sure I understand this part. The CONFIG_CAN_LEDS support for
showing the state of the interface is implemented via hooking into the
ndo_open/ndo_stop callbacks, and does not look at or touch the
__LINK_STATE_NOCARRIER bit at all.
Other than via the netdev LED trigger I don't think one can even observe
the slightly odd initial state of the __LINK_STATE_NOCARRIER bit for CAN
devices, which is why I framed this as a fix purely to allow the netdev
trigger to be a closer drop-in replacement for CONFIG_CAN_LEDS.
Rasmus
^ permalink raw reply
* RE: [PATCH net-next 04/16] qlge: Remove bq_desc.maplen
From: Manish Chopra @ 2019-06-26 9:31 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-4-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [PATCH net-next 04/16] qlge: Remove bq_desc.maplen
>
> The size of the mapping is known statically in all cases, there's no need to save
> it at runtime. Remove this member.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/qlogic/qlge/qlge.h | 1 -
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 43 +++++++-------------
> 2 files changed, 15 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h
> b/drivers/net/ethernet/qlogic/qlge/qlge.h
> index ba61b4559dd6..f32da8c7679f 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge.h
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
> @@ -1373,7 +1373,6 @@ struct bq_desc {
> __le64 *addr;
> u32 index;
> DEFINE_DMA_UNMAP_ADDR(mapaddr);
> - DEFINE_DMA_UNMAP_LEN(maplen);
> };
>
> #define QL_TXQ_IDX(qdev, skb) (smp_processor_id()%(qdev->tx_ring_count))
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 9df06ad3fb93..25dbaa9cc55d 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -1108,8 +1108,6 @@ static void ql_update_lbq(struct ql_adapter *qdev,
> struct rx_ring *rx_ring)
> map = lbq_desc->p.pg_chunk.map +
> lbq_desc->p.pg_chunk.offset;
> dma_unmap_addr_set(lbq_desc, mapaddr, map);
> - dma_unmap_len_set(lbq_desc, maplen,
> - qdev->lbq_buf_size);
> *lbq_desc->addr = cpu_to_le64(map);
>
> pci_dma_sync_single_for_device(qdev->pdev, map,
> @@ -1177,8 +1175,6 @@ static void ql_update_sbq(struct ql_adapter *qdev,
> struct rx_ring *rx_ring)
> return;
> }
> dma_unmap_addr_set(sbq_desc, mapaddr,
> map);
> - dma_unmap_len_set(sbq_desc, maplen,
> - rx_ring->sbq_buf_size);
> *sbq_desc->addr = cpu_to_le64(map);
> }
>
> @@ -1598,14 +1594,14 @@ static void ql_process_mac_rx_skb(struct
> ql_adapter *qdev,
>
> pci_dma_sync_single_for_cpu(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
>
> skb_put_data(new_skb, skb->data, length);
>
> pci_dma_sync_single_for_device(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb = new_skb;
>
> @@ -1727,8 +1723,7 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> sbq_desc = ql_get_curr_sbuf(rx_ring);
> pci_unmap_single(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> - PCI_DMA_FROMDEVICE);
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb = sbq_desc->p.skb;
> ql_realign_skb(skb, hdr_len);
> skb_put(skb, hdr_len);
> @@ -1758,19 +1753,15 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> */
> sbq_desc = ql_get_curr_sbuf(rx_ring);
> pci_dma_sync_single_for_cpu(qdev->pdev,
> - dma_unmap_addr
> - (sbq_desc, mapaddr),
> - dma_unmap_len
> - (sbq_desc, maplen),
> +
> dma_unmap_addr(sbq_desc,
> + mapaddr),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb_put_data(skb, sbq_desc->p.skb->data, length);
> pci_dma_sync_single_for_device(qdev->pdev,
> - dma_unmap_addr
> - (sbq_desc,
> - mapaddr),
> - dma_unmap_len
> - (sbq_desc,
> - maplen),
> +
> dma_unmap_addr(sbq_desc,
> + mapaddr),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> } else {
> netif_printk(qdev, rx_status, KERN_DEBUG, qdev-
> >ndev, @@ -1781,10 +1772,8 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> ql_realign_skb(skb, length);
> skb_put(skb, length);
> pci_unmap_single(qdev->pdev,
> - dma_unmap_addr(sbq_desc,
> - mapaddr),
> - dma_unmap_len(sbq_desc,
> - maplen),
> + dma_unmap_addr(sbq_desc,
> mapaddr),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> sbq_desc->p.skb = NULL;
> }
> @@ -1822,9 +1811,8 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> return NULL;
> }
> pci_unmap_page(qdev->pdev,
> - dma_unmap_addr(lbq_desc,
> - mapaddr),
> - dma_unmap_len(lbq_desc, maplen),
> + dma_unmap_addr(lbq_desc, mapaddr),
> + qdev->lbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb_reserve(skb, NET_IP_ALIGN);
> netif_printk(qdev, rx_status, KERN_DEBUG, qdev-
> >ndev, @@ -1858,8 +1846,7 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> sbq_desc = ql_get_curr_sbuf(rx_ring);
> pci_unmap_single(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> - PCI_DMA_FROMDEVICE);
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> if (!(ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HS)) {
> /*
> * This is an non TCP/UDP IP frame, so @@ -2820,7
> +2807,7 @@ static void ql_free_sbq_buffers(struct ql_adapter *qdev, struct
> rx_ring *rx_ring
> if (sbq_desc->p.skb) {
> pci_unmap_single(qdev->pdev,
> dma_unmap_addr(sbq_desc,
> mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> dev_kfree_skb(sbq_desc->p.skb);
> sbq_desc->p.skb = NULL;
> --
> 2.21.0
Acked-by: Manish Chopra <manishc@marvell.com>
^ permalink raw reply
* RE: [EXT] [PATCH net-next 05/16] qlge: Remove rx_ring.sbq_buf_size
From: Manish Chopra @ 2019-06-26 9:36 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-5-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [EXT] [PATCH net-next 05/16] qlge: Remove rx_ring.sbq_buf_size
>
> External Email
>
> ----------------------------------------------------------------------
> Tx rings have sbq_buf_size = 0 but there's no case where the code actually
> tests on that value. We can remove sbq_buf_size and use a constant instead.
>
Seems relevant to RX ring, not the TX ring ?
^ permalink raw reply
* Re: [RFC PATCH v4 net-next 06/11] net: ethernet: ti: introduce cpsw switchdev based driver part 1 - dual-emac
From: Ivan Khoronzhuk @ 2019-06-26 9:58 UTC (permalink / raw)
To: Grygorii Strashko
Cc: netdev, Ilias Apalodimas, Andrew Lunn, David S . Miller,
Jiri Pirko, Florian Fainelli, Sekhar Nori, linux-kernel,
linux-omap, Murali Karicheri, Ivan Vecera, Rob Herring,
devicetree
In-Reply-To: <20190621181314.20778-7-grygorii.strashko@ti.com>
Hi Grygorii
Too much code, but I've tried pass thru.
Probably expectation the devlink to be reviewed, but several
common replies that should be reflected in non RFC v.
On Fri, Jun 21, 2019 at 09:13:09PM +0300, Grygorii Strashko wrote:
>From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
>Part 1:
> Introduce basic CPSW dual_mac driver (cpsw_new.c) which is operating in
>dual-emac mode by default, thus working as 2 individual network interfaces.
>Main differences from legacy CPSW driver are:
>
> - optimized promiscuous mode: The P0_UNI_FLOOD (both ports) is enabled in
>addition to ALLMULTI (current port) instead of ALE_BYPASS. So, Ports in
>promiscuous mode will keep possibility of mcast and vlan filtering, which
>is provides significant benefits when ports are joined to the same bridge,
>but without enabling "switch" mode, or to different bridges.
> - learning disabled on ports as it make not too much sense for
> segregated ports - no forwarding in HW.
> - enabled basic support for devlink.
>
> devlink dev show
> platform/48484000.ethernet_switch
>
> devlink dev param show
> platform/48484000.ethernet_switch:
> name ale_bypass type driver-specific
> values:
> cmode runtime value false
>
> - "ale_bypass" devlink driver parameter allows to enable
>ALE_CONTROL(4).BYPASS mode for debug purposes.
> - updated DT bindings.
>
>Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>---
> drivers/net/ethernet/ti/Kconfig | 19 +-
> drivers/net/ethernet/ti/Makefile | 2 +
> drivers/net/ethernet/ti/cpsw_new.c | 1555 +++++++++++++++++++++++++++
> drivers/net/ethernet/ti/cpsw_priv.c | 8 +-
> drivers/net/ethernet/ti/cpsw_priv.h | 12 +-
> 5 files changed, 1591 insertions(+), 5 deletions(-)
> create mode 100644 drivers/net/ethernet/ti/cpsw_new.c
>
[...]
>+
>+static void cpsw_rx_handler(void *token, int len, int status)
>+{
>+ struct sk_buff *skb = token;
>+ struct cpsw_common *cpsw;
>+ struct net_device *ndev;
>+ struct sk_buff *new_skb;
>+ struct cpsw_priv *priv;
>+ struct cpdma_chan *ch;
>+ int ret = 0, port;
>+
>+ ndev = skb->dev;
>+ cpsw = ndev_to_cpsw(ndev);
>+
>+ port = CPDMA_RX_SOURCE_PORT(status);
>+ if (port) {
>+ ndev = cpsw->slaves[--port].ndev;
>+ skb->dev = ndev;
>+ }
>+
>+ if (unlikely(status < 0) || unlikely(!netif_running(ndev))) {
>+ /* In dual emac mode check for all interfaces */
>+ if (cpsw->usage_count && status >= 0) {
>+ /* The packet received is for the interface which
>+ * is already down and the other interface is up
>+ * and running, instead of freeing which results
>+ * in reducing of the number of rx descriptor in
>+ * DMA engine, requeue skb back to cpdma.
>+ */
>+ new_skb = skb;
>+ goto requeue;
>+ }
>+
>+ /* the interface is going down, skbs are purged */
>+ dev_kfree_skb_any(skb);
>+ return;
>+ }
>+
>+ priv = netdev_priv(ndev);
>+
>+ new_skb = netdev_alloc_skb_ip_align(ndev, cpsw->rx_packet_max);
>+ if (new_skb) {
>+ skb_copy_queue_mapping(new_skb, skb);
>+ skb_put(skb, len);
>+ if (status & CPDMA_RX_VLAN_ENCAP)
>+ cpsw_rx_vlan_encap(skb);
>+ if (priv->rx_ts_enabled)
>+ cpts_rx_timestamp(cpsw->cpts, skb);
>+ skb->protocol = eth_type_trans(skb, ndev);
>+ netif_receive_skb(skb);
>+ ndev->stats.rx_bytes += len;
>+ ndev->stats.rx_packets++;
>+ /* CPDMA stores skb in internal CPPI RAM (SRAM) which belongs
>+ * to DEV MMIO space. Kmemleak does not scan IO memory and so
>+ * reports memory leaks.
>+ * see commit 254a49d5139a ('drivers: net: cpsw: fix kmemleak
>+ * false-positive reports for sk buffers') for details.
>+ */
>+ kmemleak_not_leak(new_skb);
>+ } else {
>+ ndev->stats.rx_dropped++;
>+ new_skb = skb;
>+ }
>+
>+requeue:
----
>+ if (netif_dormant(ndev)) {
>+ dev_kfree_skb_any(new_skb);
>+ return;
>+ }
---
drop above, no need any more
>+
>+ ch = cpsw->rxv[skb_get_queue_mapping(new_skb)].ch;
>+ ret = cpdma_chan_submit(ch, new_skb, new_skb->data,
>+ skb_tailroom(new_skb), 0);
>+ if (WARN_ON(ret < 0))
>+ dev_kfree_skb_any(new_skb);
Here were a little changes last time, looks like:
ret = cpdma_chan_submit(ch, new_skb, new_skb->data,
skb_tailroom(new_skb), 0);
if (ret < 0) {
WARN_ON(ret == -ENOMEM);
dev_kfree_skb_any(new_skb);
}
>+}
>+
[...]
>+
>+static void cpsw_init_host_port(struct cpsw_priv *priv)
>+{
>+ struct cpsw_common *cpsw = priv->cpsw;
>+ u32 control_reg;
>+
>+ /* soft reset the controller and initialize ale */
>+ soft_reset("cpsw", &cpsw->regs->soft_reset);
>+ cpsw_ale_start(cpsw->ale);
>+
>+ /* switch to vlan unaware mode */
>+ cpsw_ale_control_set(cpsw->ale, HOST_PORT_NUM, ALE_VLAN_AWARE,
>+ CPSW_ALE_VLAN_AWARE);
>+ control_reg = readl(&cpsw->regs->control);
>+ control_reg |= CPSW_VLAN_AWARE | CPSW_RX_VLAN_ENCAP;
>+ writel(control_reg, &cpsw->regs->control);
>+
>+ /* setup host port priority mapping */
>+ writel_relaxed(CPDMA_TX_PRIORITY_MAP,
>+ &cpsw->host_port_regs->cpdma_tx_pri_map);
>+ writel_relaxed(0, &cpsw->host_port_regs->cpdma_rx_chan_map);
----
>+
>+ /* disable priority elevation */
>+ writel_relaxed(0, &cpsw->regs->ptype);
>+
>+ /* enable statistics collection only on all ports */
>+ writel_relaxed(0x7, &cpsw->regs->stat_port_en);
>+
>+ /* Enable internal fifo flow control */
>+ writel(0x7, &cpsw->regs->flow_control);
---
Would be nice to do the same in old driver.
I mean moving it from ndo_open
Also were thoughts about this.
>+
>+ cpsw_init_host_port_dual_mac(priv);
>+
>+ cpsw_ale_control_set(cpsw->ale, HOST_PORT_NUM,
>+ ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
>+}
>+
[...]
>+
>+static int cpsw_ndo_open(struct net_device *ndev)
>+{
>+ struct cpsw_priv *priv = netdev_priv(ndev);
>+ struct cpsw_common *cpsw = priv->cpsw;
>+ int ret;
>+
>+ cpsw_info(priv, ifdown, "starting ndev\n");
>+ ret = pm_runtime_get_sync(cpsw->dev);
>+ if (ret < 0) {
>+ pm_runtime_put_noidle(cpsw->dev);
>+ return ret;
>+ }
>+
>+ netif_carrier_off(ndev);
>+
>+ /* Notify the stack of the actual queue counts. */
>+ ret = netif_set_real_num_tx_queues(ndev, cpsw->tx_ch_num);
>+ if (ret) {
>+ dev_err(priv->dev, "cannot set real number of tx queues\n");
>+ goto err_cleanup;
>+ }
>+
>+ ret = netif_set_real_num_rx_queues(ndev, cpsw->rx_ch_num);
>+ if (ret) {
>+ dev_err(priv->dev, "cannot set real number of rx queues\n");
>+ goto err_cleanup;
>+ }
>+
>+ /* Initialize host and slave ports */
>+ if (!cpsw->usage_count)
>+ cpsw_init_host_port(priv);
>+ cpsw_slave_open(&cpsw->slaves[priv->emac_port - 1], priv);
>+
>+ /* initialize shared resources for every ndev */
>+ if (!cpsw->usage_count) {
>+ ret = cpsw_fill_rx_channels(priv);
>+ if (ret < 0)
>+ goto err_cleanup;
>+
>+ if (cpts_register(cpsw->cpts))
>+ dev_err(priv->dev, "error registering cpts device\n");
>+
>+ napi_enable(&cpsw->napi_rx);
>+ napi_enable(&cpsw->napi_tx);
>+
>+ if (cpsw->tx_irq_disabled) {
>+ cpsw->tx_irq_disabled = false;
>+ enable_irq(cpsw->irqs_table[1]);
>+ }
>+
>+ if (cpsw->rx_irq_disabled) {
>+ cpsw->rx_irq_disabled = false;
>+ enable_irq(cpsw->irqs_table[0]);
>+ }
>+ }
>+
>+ cpsw_restore(priv);
>+
>+ /* Enable Interrupt pacing if configured */
>+ if (cpsw->coal_intvl != 0) {
>+ struct ethtool_coalesce coal;
>+
>+ coal.rx_coalesce_usecs = cpsw->coal_intvl;
>+ cpsw_set_coalesce(ndev, &coal);
>+ }
>+
>+ cpdma_ctlr_start(cpsw->dma);
>+ cpsw_intr_enable(cpsw);
>+ cpsw->usage_count++;
>+
>+ return 0;
>+
>+err_cleanup:
>+ cpdma_ctlr_stop(cpsw->dma);
Here were a little changes also:
Now looks like:
if (!cpsw->usage_count) {
cpdma_ctlr_stop(cpsw->dma)
}
>+ cpsw_slave_stop(&cpsw->slaves[priv->emac_port - 1], priv);
>+ pm_runtime_put_sync(cpsw->dev);
>+ netif_carrier_off(priv->ndev);
>+ return ret;
>+}
>+
[...]
>+
>+static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
>+ struct net_device *ndev)
>+{
>+ struct cpsw_priv *priv = netdev_priv(ndev);
>+ struct cpsw_common *cpsw = priv->cpsw;
>+ struct cpts *cpts = cpsw->cpts;
>+ struct netdev_queue *txq;
>+ struct cpdma_chan *txch;
>+ int ret, q_idx;
>+
>+ if (skb_padto(skb, CPSW_MIN_PACKET_SIZE)) {
>+ cpsw_err(priv, tx_err, "packet pad failed\n");
>+ ndev->stats.tx_dropped++;
>+ return NET_XMIT_DROP;
>+ }
>+
>+ if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
>+ priv->tx_ts_enabled && cpts_can_timestamp(cpts, skb))
>+ skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
>+
>+ q_idx = skb_get_queue_mapping(skb);
>+ if (q_idx >= cpsw->tx_ch_num)
>+ q_idx = q_idx % cpsw->tx_ch_num;
>+
>+ txch = cpsw->txv[q_idx].ch;
>+ txq = netdev_get_tx_queue(ndev, q_idx);
>+ skb_tx_timestamp(skb);
>+ ret = cpdma_chan_submit(txch, skb, skb->data, skb->len,
>+ priv->emac_port);
------------
>+ if (unlikely(ret != 0)) {
>+ cpsw_err(priv, tx_err, "desc submit failed\n");
>+ goto fail;
>+ }
>+
>+ /* If there is no more tx desc left free then we need to
>+ * tell the kernel to stop sending us tx frames.
>+ */
>+ if (unlikely(!cpdma_check_free_tx_desc(txch))) {
>+ netif_tx_stop_queue(txq);
>+
>+ /* Barrier, so that stop_queue visible to other cpus */
>+ smp_mb__after_atomic();
>+
>+ if (cpdma_check_free_tx_desc(txch))
>+ netif_tx_wake_queue(txq);
>+ }
>+
>+ return NETDEV_TX_OK;
>+fail:
>+ ndev->stats.tx_dropped++;
>+ netif_tx_stop_queue(txq);
>+
>+ /* Barrier, so that stop_queue visible to other cpus */
>+ smp_mb__after_atomic();
>+
>+ if (cpdma_check_free_tx_desc(txch))
>+ netif_tx_wake_queue(txq);
>+
>+ return NETDEV_TX_BUSY;
>+}
-------------
I have a proposition, here were no reason lastly, but now maybe
it can be optimized a little. Smth like, replace above on:
if (unlikely(ret != 0)) {
cpsw_err(priv, tx_err, "desc submit failed\n");
ndev->stats.tx_dropped++;
ret = NETDEV_TX_BUSY;
goto fail
}
ret = NETDEV_TX_OK;
/* If there is no more tx desc left free then we need to
* tell the kernel to stop sending us tx frames.
*/
if (unlikely(cpdma_check_free_tx_desc(txch)))
return ret;
fail:
netif_tx_stop_queue(txq);
/* Barrier, so that stop_queue visible to other cpus */
smp_mb__after_atomic();
if (cpdma_check_free_tx_desc(txch))
netif_tx_wake_queue(txq);
return ret;
Result: minus 6 lines and less dupes.
>+
>+
[...]
>+static int cpsw_probe(struct platform_device *pdev)
>+{
>+ const struct soc_device_attribute *soc;
>+ struct device *dev = &pdev->dev;
>+ struct resource *ss_res;
>+ struct cpsw_common *cpsw;
>+ struct gpio_descs *mode;
>+ void __iomem *ss_regs;
>+ int ret = 0, ch;
>+ struct clk *clk;
>+ int irq;
>+
>+ cpsw = devm_kzalloc(dev, sizeof(struct cpsw_common), GFP_KERNEL);
>+ if (!cpsw)
>+ return -ENOMEM;
>+
>+ cpsw_slave_index = cpsw_slave_index_priv;
>+
>+ cpsw->dev = dev;
>+
>+ cpsw->slaves = devm_kcalloc(dev,
>+ CPSW_SLAVE_PORTS_NUM,
>+ sizeof(struct cpsw_slave),
>+ GFP_KERNEL);
>+ if (!cpsw->slaves)
>+ return -ENOMEM;
>+
>+ mode = devm_gpiod_get_array_optional(dev, "mode", GPIOD_OUT_LOW);
>+ if (IS_ERR(mode)) {
>+ ret = PTR_ERR(mode);
>+ dev_err(dev, "gpio request failed, ret %d\n", ret);
>+ return ret;
>+ }
>+
>+ clk = devm_clk_get(dev, "fck");
>+ if (IS_ERR(clk)) {
>+ ret = PTR_ERR(clk);
>+ dev_err(dev, "fck is not found %d\n", ret);
>+ return ret;
>+ }
>+ cpsw->bus_freq_mhz = clk_get_rate(clk) / 1000000;
>+
>+ ss_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>+ ss_regs = devm_ioremap_resource(dev, ss_res);
>+ if (IS_ERR(ss_regs)) {
>+ ret = PTR_ERR(ss_regs);
>+ return ret;
>+ }
>+ cpsw->regs = ss_regs;
>+
>+ irq = platform_get_irq_byname(pdev, "rx");
>+ if (irq < 0)
>+ return irq;
>+ cpsw->irqs_table[0] = irq;
>+
>+ irq = platform_get_irq_byname(pdev, "tx");
>+ if (irq < 0)
>+ return irq;
>+ cpsw->irqs_table[1] = irq;
>+
>+ platform_set_drvdata(pdev, cpsw);
set cpsw, but below ndev
>+ /* This may be required here for child devices. */
>+ pm_runtime_enable(dev);
>+
>+ /* Need to enable clocks with runtime PM api to access module
>+ * registers
>+ */
>+ ret = pm_runtime_get_sync(dev);
>+ if (ret < 0) {
>+ pm_runtime_put_noidle(dev);
>+ pm_runtime_disable(dev);
>+ return ret;
>+ }
>+
>+ ret = cpsw_probe_dt(cpsw);
>+ if (ret)
>+ goto clean_dt_ret;
>+
>+ soc = soc_device_match(cpsw_soc_devices);
>+ if (soc)
>+ cpsw->quirk_irq = 1;
>+
>+ cpsw->rx_packet_max = rx_packet_max;
>+ cpsw->descs_pool_size = descs_pool_size;
>+
>+ ret = cpsw_init_common(cpsw, ss_regs, ale_ageout,
>+ (u32 __force)ss_res->start + CPSW2_BD_OFFSET,
>+ descs_pool_size);
>+ if (ret)
>+ goto clean_dt_ret;
>+
>+ cpsw->wr_regs = cpsw->version == CPSW_VERSION_1 ?
>+ ss_regs + CPSW1_WR_OFFSET :
>+ ss_regs + CPSW2_WR_OFFSET;
>+
>+ ch = cpsw->quirk_irq ? 0 : 7;
>+ cpsw->txv[0].ch = cpdma_chan_create(cpsw->dma, ch, cpsw_tx_handler, 0);
>+ if (IS_ERR(cpsw->txv[0].ch)) {
>+ dev_err(dev, "error initializing tx dma channel\n");
>+ ret = PTR_ERR(cpsw->txv[0].ch);
>+ goto clean_cpts;
>+ }
>+
>+ cpsw->rxv[0].ch = cpdma_chan_create(cpsw->dma, 0, cpsw_rx_handler, 1);
>+ if (IS_ERR(cpsw->rxv[0].ch)) {
>+ dev_err(dev, "error initializing rx dma channel\n");
>+ ret = PTR_ERR(cpsw->rxv[0].ch);
>+ goto clean_cpts;
>+ }
>+ cpsw_split_res(cpsw);
>+
>+ /* setup netdevs */
>+ ret = cpsw_create_ports(cpsw);
>+ if (ret)
>+ goto clean_unregister_netdev;
>+
>+ /* Grab RX and TX IRQs. Note that we also have RX_THRESHOLD and
>+ * MISC IRQs which are always kept disabled with this driver so
>+ * we will not request them.
>+ *
>+ * If anyone wants to implement support for those, make sure to
>+ * first request and append them to irqs_table array.
>+ */
>+
>+ ret = devm_request_irq(dev, cpsw->irqs_table[0], cpsw_rx_interrupt,
>+ 0, dev_name(dev), cpsw);
>+ if (ret < 0) {
>+ dev_err(dev, "error attaching irq (%d)\n", ret);
>+ goto clean_unregister_netdev;
>+ }
>+
>+ ret = devm_request_irq(dev, cpsw->irqs_table[1], cpsw_tx_interrupt,
>+ 0, dev_name(dev), cpsw);
>+ if (ret < 0) {
>+ dev_err(dev, "error attaching irq (%d)\n", ret);
>+ goto clean_unregister_netdev;
>+ }
>+
>+ ret = cpsw_register_devlink(cpsw);
>+ if (ret)
>+ goto clean_unregister_netdev;
>+
>+ dev_notice(dev, "initialized (regs %pa, pool size %d) hw_ver:%08X %d.%d (%d)\n",
>+ &ss_res->start, descs_pool_size,
>+ cpsw->version, CPSW_MAJOR_VERSION(cpsw->version),
>+ CPSW_MINOR_VERSION(cpsw->version),
>+ CPSW_RTL_VERSION(cpsw->version));
>+
>+ pm_runtime_put(dev);
>+
>+ return 0;
>+
>+clean_unregister_netdev:
>+ cpsw_unregister_ports(cpsw);
>+clean_cpts:
>+ cpts_release(cpsw->cpts);
>+ cpdma_ctlr_destroy(cpsw->dma);
>+clean_dt_ret:
>+ cpsw_remove_dt(cpsw);
>+ pm_runtime_put_sync(dev);
>+ pm_runtime_disable(dev);
>+ return ret;
>+}
>+
>+static int cpsw_remove(struct platform_device *pdev)
>+{
>+ struct net_device *ndev = platform_get_drvdata(pdev);
now it's cpsw, as in probe?
>+ struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
>+ int ret;
>+
>+ ret = pm_runtime_get_sync(&pdev->dev);
>+ if (ret < 0) {
>+ pm_runtime_put_noidle(&pdev->dev);
>+ return ret;
>+ }
>+
>+ cpsw_unregister_devlink(cpsw);
>+ cpsw_unregister_ports(cpsw);
>+
>+ cpts_release(cpsw->cpts);
>+ cpdma_ctlr_destroy(cpsw->dma);
>+ cpsw_remove_dt(cpsw);
>+ pm_runtime_put_sync(&pdev->dev);
>+ pm_runtime_disable(&pdev->dev);
>+ return 0;
>+}
>+
[...]
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: KASAN: use-after-free Write in validate_chain
From: syzbot @ 2019-06-26 10:04 UTC (permalink / raw)
To: ast, daniel, john.fastabend, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <0000000000000c4e3e058bd5008d@google.com>
syzbot has bisected this bug to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1555b795a00000
start commit: abf02e29 Merge tag 'pm-5.2-rc6' of git://git.kernel.org/pu..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=1755b795a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1355b795a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=28ec3437a5394ee0
dashboard link: https://syzkaller.appspot.com/bug?extid=55c548ad445cef6063ab
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=128ce13aa00000
Reported-by: syzbot+55c548ad445cef6063ab@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* [PATCH net] ipv4: fix suspicious RCU usage in fib_dump_info_fnhe()
From: Eric Dumazet @ 2019-06-26 10:04 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Stefano Brivio, David Ahern,
syzbot
sysbot reported that we lack appropriate rcu_read_lock()
protection in fib_dump_info_fnhe()
net/ipv4/route.c:2875 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by syz-executor609/8966:
#0: 00000000b7dbe288 (rtnl_mutex){+.+.}, at: netlink_dump+0xe7/0xfb0 net/netlink/af_netlink.c:2199
stack backtrace:
CPU: 0 PID: 8966 Comm: syz-executor609 Not tainted 5.2.0-rc5+ #43
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
lockdep_rcu_suspicious+0x153/0x15d kernel/locking/lockdep.c:5250
fib_dump_info_fnhe+0x9d9/0x1080 net/ipv4/route.c:2875
fn_trie_dump_leaf net/ipv4/fib_trie.c:2141 [inline]
fib_table_dump+0x64a/0xd00 net/ipv4/fib_trie.c:2175
inet_dump_fib+0x83c/0xa90 net/ipv4/fib_frontend.c:1004
rtnl_dump_all+0x295/0x490 net/core/rtnetlink.c:3445
netlink_dump+0x558/0xfb0 net/netlink/af_netlink.c:2244
__netlink_dump_start+0x5b1/0x7d0 net/netlink/af_netlink.c:2352
netlink_dump_start include/linux/netlink.h:226 [inline]
rtnetlink_rcv_msg+0x73d/0xb00 net/core/rtnetlink.c:5182
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5237
netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:646 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:665
sock_write_iter+0x27c/0x3e0 net/socket.c:994
call_write_iter include/linux/fs.h:1872 [inline]
new_sync_write+0x4d3/0x770 fs/read_write.c:483
__vfs_write+0xe1/0x110 fs/read_write.c:496
vfs_write+0x20c/0x580 fs/read_write.c:558
ksys_write+0x14f/0x290 fs/read_write.c:611
__do_sys_write fs/read_write.c:623 [inline]
__se_sys_write fs/read_write.c:620 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:620
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4401b9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc8e134978 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004401b9
RDX: 000000000000001c RSI: 0000000020000000 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 0000000000000010 R11: 0000000000000246 R12: 0000000000401a40
R13: 0000000000401ad0 R14: 0000000000000000 R15: 0000000000000000
Fixes: ee28906fd7a1 ("ipv4: Dump route exceptions if requested")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Stefano Brivio <sbrivio@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
net/ipv4/route.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 6aee412a68bdd3c24a6a0eb9883e04b7a83998e0..59670fafcd2612b94c237cbe30109adb196cf3f0 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2872,12 +2872,13 @@ int fib_dump_info_fnhe(struct sk_buff *skb, struct netlink_callback *cb,
if (nhc->nhc_flags & RTNH_F_DEAD)
continue;
+ rcu_read_lock();
bucket = rcu_dereference(nhc->nhc_exceptions);
- if (!bucket)
- continue;
-
- err = fnhe_dump_bucket(net, skb, cb, table_id, bucket, genid,
- fa_index, fa_start);
+ err = 0;
+ if (bucket)
+ err = fnhe_dump_bucket(net, skb, cb, table_id, bucket,
+ genid, fa_index, fa_start);
+ rcu_read_unlock();
if (err)
return err;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net] ipv6: fix suspicious RCU usage in rt6_dump_route()
From: Eric Dumazet @ 2019-06-26 10:05 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Stefano Brivio, David Ahern
syzbot reminded us that rt6_nh_dump_exceptions() needs to be called
with rcu_read_lock()
net/ipv6/route.c:1593 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by syz-executor609/8966:
#0: 00000000b7dbe288 (rtnl_mutex){+.+.}, at: netlink_dump+0xe7/0xfb0 net/netlink/af_netlink.c:2199
#1: 00000000f2d87c21 (&(&tb->tb6_lock)->rlock){+...}, at: spin_lock_bh include/linux/spinlock.h:343 [inline]
#1: 00000000f2d87c21 (&(&tb->tb6_lock)->rlock){+...}, at: fib6_dump_table.isra.0+0x37e/0x570 net/ipv6/ip6_fib.c:533
stack backtrace:
CPU: 0 PID: 8966 Comm: syz-executor609 Not tainted 5.2.0-rc5+ #43
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
lockdep_rcu_suspicious+0x153/0x15d kernel/locking/lockdep.c:5250
fib6_nh_get_excptn_bucket+0x18e/0x1b0 net/ipv6/route.c:1593
rt6_nh_dump_exceptions+0x45/0x4d0 net/ipv6/route.c:5541
rt6_dump_route+0x904/0xc50 net/ipv6/route.c:5640
fib6_dump_node+0x168/0x280 net/ipv6/ip6_fib.c:467
fib6_walk_continue+0x4a9/0x8e0 net/ipv6/ip6_fib.c:1986
fib6_walk+0x9d/0x100 net/ipv6/ip6_fib.c:2034
fib6_dump_table.isra.0+0x38a/0x570 net/ipv6/ip6_fib.c:534
inet6_dump_fib+0x93c/0xb00 net/ipv6/ip6_fib.c:624
rtnl_dump_all+0x295/0x490 net/core/rtnetlink.c:3445
netlink_dump+0x558/0xfb0 net/netlink/af_netlink.c:2244
__netlink_dump_start+0x5b1/0x7d0 net/netlink/af_netlink.c:2352
netlink_dump_start include/linux/netlink.h:226 [inline]
rtnetlink_rcv_msg+0x73d/0xb00 net/core/rtnetlink.c:5182
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5237
netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:646 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:665
sock_write_iter+0x27c/0x3e0 net/socket.c:994
call_write_iter include/linux/fs.h:1872 [inline]
new_sync_write+0x4d3/0x770 fs/read_write.c:483
__vfs_write+0xe1/0x110 fs/read_write.c:496
vfs_write+0x20c/0x580 fs/read_write.c:558
ksys_write+0x14f/0x290 fs/read_write.c:611
__do_sys_write fs/read_write.c:623 [inline]
__se_sys_write fs/read_write.c:620 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:620
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4401b9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc8e134978 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004401b9
RDX: 000000000000001c RSI: 0000000020000000 RDI: 00
Fixes: 1e47b4837f3b ("ipv6: Dump route exceptions if requested")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Stefano Brivio <sbrivio@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
---
net/ipv6/route.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index be5e65c97652d0c34d209f85c8295d6faf871990..c59e97cf9d25da3084098572896178b71fb28fe6 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5632,6 +5632,7 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg, unsigned int skip)
.count = 0 };
int err;
+ rcu_read_lock();
if (rt->nh) {
err = nexthop_for_each_fib6_nh(rt->nh,
rt6_nh_dump_exceptions,
@@ -5639,6 +5640,7 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg, unsigned int skip)
} else {
err = rt6_nh_dump_exceptions(rt->fib6_nh, &w);
}
+ rcu_read_unlock();
if (err)
return count += w.count;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: [PATCH net] ipv4: fix suspicious RCU usage in fib_dump_info_fnhe()
From: Eric Dumazet @ 2019-06-26 10:09 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Stefano Brivio, David Ahern, syzbot
In-Reply-To: <20190626100450.217106-1-edumazet@google.com>
On Wed, Jun 26, 2019 at 12:04 PM Eric Dumazet <edumazet@google.com> wrote:
>
> sysbot reported that we lack appropriate rcu_read_lock()
> protection in fib_dump_info_fnhe()
This is for net-next tree, sorry for the confusion.
^ permalink raw reply
* Re: [PATCH net] ipv6: fix suspicious RCU usage in rt6_dump_route()
From: Eric Dumazet @ 2019-06-26 10:09 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Stefano Brivio, David Ahern
In-Reply-To: <20190626100528.218097-1-edumazet@google.com>
On Wed, Jun 26, 2019 at 12:05 PM Eric Dumazet <edumazet@google.com> wrote:
>
> syzbot reminded us that rt6_nh_dump_exceptions() needs to be called
> with rcu_read_lock()
Same remark : This is for net-next tree.
^ permalink raw reply
* BUG: unable to handle kernel paging request in tls_prots
From: syzbot @ 2019-06-26 10:17 UTC (permalink / raw)
To: ast, bpf, daniel, davem, edumazet, john.fastabend, kafai, kuznet,
linux-kernel, netdev, songliubraving, syzkaller-bugs, yhs,
yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: 904d88d7 qmi_wwan: Fix out-of-bounds read
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=14a8b865a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=137ec2016ea3870d
dashboard link: https://syzkaller.appspot.com/bug?extid=4207c7f3a443366d8aa2
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15576c71a00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4207c7f3a443366d8aa2@syzkaller.appspotmail.com
BUG: unable to handle page fault for address: 000000004125973f
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD 92e1a067 P4D 92e1a067 PUD 0
Thread overran stack, or stack corrupted
Oops: 0002 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 9943 Comm: blkid Not tainted 5.2.0-rc5+ #62
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:tls_prots+0x1a8a/0x3520
Code: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 70 0d 0f 86 ff ff ff ff 40 5d <28> 86 ff ff ff
ff 60 6e 28 86 ff ff ff ff 00 de ed 85 ff ff ff ff
RSP: 0018:ffff888079bc7c10 EFLAGS: 00010082
RAX: ffff88808c5226c0 RBX: ffff88808cb36100 RCX: 1ffffffff116885c
RDX: 1ffff110118a44d8 RSI: 0000000041259740 RDI: ffff88808c523a70
RBP: ffff888079bc7c38 R08: ffff88808c5226c0 R09: ffffed1015d26c70
R10: ffffed1015d26c6f R11: ffff8880ae93637b R12: ffffffff860dd760
R13: ffffffff860dda75 R14: ffff888079bc7c08 R15: ffffffff8b1da9a0
FS: 00007fa341259740(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000004125973f CR3: 00000000907f6000 CR4: 00000000001406e0
Call Trace:
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
WARNING: kernel stack frame pointer at 00000000e809d5dc in blkid:9943 has
bad value 000000009844d018
unwind stack type:0 next_sp:000000005f927a44 mask:0x2 graph_idx:0
000000002401cb95: ffff888079bc7c68 (0xffff888079bc7c68)
00000000c24ed912: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000db5b548d: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000479aff76: ffff88808cb36100 (0xffff88808cb36100)
000000004473e5aa: 0000000000000000 ...
000000009d12a602: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000ba44e25c: ffff888079bc7c98 (0xffff888079bc7c98)
00000000e1eb23e4: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000f7d29ffb: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000dc871c27: ffff88808cb36100 (0xffff88808cb36100)
000000003824545e: 0000000000000000 ...
000000008a182c7c: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000001c7a51e7: ffff888079bc7cc8 (0xffff888079bc7cc8)
0000000049da0237: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000009c31c0da: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
0000000024f3129d: ffff88808cb36100 (0xffff88808cb36100)
0000000040823c88: 0000000000000000 ...
000000004c33f42e: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000fa38e5a4: ffff888079bc7cf8 (0xffff888079bc7cf8)
0000000030a46a60: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000313cd10c: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000003feafbe9: ffff88808cb36100 (0xffff88808cb36100)
00000000b2d0344c: 0000000000000000 ...
00000000a54c1af0: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000d4027d6c: ffff888079bc7d28 (0xffff888079bc7d28)
000000009ee98499: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000cc716643: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000f66028fc: ffff88808cb36100 (0xffff88808cb36100)
00000000f1a9f4a4: 0000000000000000 ...
0000000093e98749: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000069764410: ffff888079bc7d58 (0xffff888079bc7d58)
0000000099cc4d5e: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000008452556b: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000009d210107: ffff88808cb36100 (0xffff88808cb36100)
000000000b3d1c61: 0000000000000000 ...
0000000029482aba: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000000080c2ff: ffff888079bc7d88 (0xffff888079bc7d88)
000000004e072bc8: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000049c3c65: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000b89e7d9b: ffff88808cb36100 (0xffff88808cb36100)
000000004f6d4a66: 0000000000000000 ...
0000000062d2c1b7: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000064dd81c9: ffff888079bc7db8 (0xffff888079bc7db8)
000000003089fca0: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
0000000080e7f1f9: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000487c3ec3: ffff88808cb36100 (0xffff88808cb36100)
00000000291b99a5: 0000000000000000 ...
00000000879c3ace: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000088f16e91: ffff888079bc7de8 (0xffff888079bc7de8)
00000000feaf9900: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000699d8a79: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000ea800749: ffff88808cb36100 (0xffff88808cb36100)
000000002c99629d: 0000000000000000 ...
00000000f89aef64: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000008a8c9aec: ffff888079bc7e18 (0xffff888079bc7e18)
0000000050ff6d78: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000008764d630: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
0000000091e33368: ffff88808cb36100 (0xffff88808cb36100)
00000000e1a0ace5: 0000000000000000 ...
00000000cbc751ab: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000d4f9acf4: ffff888079bc7e48 (0xffff888079bc7e48)
0000000081fc1715: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000aa6f1bc2: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000d2a9e7ce: ffff88808cb36100 (0xffff88808cb36100)
000000006f96fab3: 0000000000000000 ...
00000000aeaeead6: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000f35f95f5: ffff888079bc7e78 (0xffff888079bc7e78)
000000008c846b2f: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000001c6a7237: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000799c1d50: ffff88808cb36100 (0xffff88808cb36100)
00000000447f9e03: 0000000000000000 ...
00000000ec08ab6f: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000079b88671: ffff888079bc7ea8 (0xffff888079bc7ea8)
0000000057ceab90: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000a3af49c4: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000c8233266: ffff88808cb36100 (0xffff88808cb36100)
00000000a4346829: 0000000000000000 ...
00000000a77b30e0: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000009289d9ef: ffff888079bc7ed8 (0xffff888079bc7ed8)
0000000093a94b95: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000bba52dcd: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000004ba72af3: ffff88808cb36100 (0xffff88808cb36100)
000000002713380e: 0000000000000000 ...
0000000082b4f5fe: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000fd331aa8: ffff888079bc7f08 (0xffff888079bc7f08)
00000000a125cb7a: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000b176b5fe: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
0000000021b424c8: ffff88808cb36100 (0xffff88808cb36100)
00000000c8aca3a7: 0000000000000000 ...
00000000ae0e04d5: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000218dfe2f: ffff888079bc7f38 (0xffff888079bc7f38)
000000006f81db8c: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000005415d20a: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000cd8b4d0d: ffff88808cb36100 (0xffff88808cb36100)
0000000028dde223: 0000000000000000 ...
00000000f1bac7a7: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000819b4e09: ffff888079bc7f68 (0xffff888079bc7f68)
0000000040faf187: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000d40fc060: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000002efea550: ffff88808cb36100 (0xffff88808cb36100)
0000000056721274: 0000000000000000 ...
000000008c6a3097: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000039ab3cf0: ffff888079bc7f98 (0xffff888079bc7f98)
00000000b825c1f0: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000005d65596f: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000a0cc451f: ffff88808cb36100 (0xffff88808cb36100)
00000000e7f05b41: 0000000000000000 ...
0000000049404ce5: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000001a81ddbb: ffff888079bc7fc8 (0xffff888079bc7fc8)
000000003afd376f: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
0000000067da2263: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000951815ed: ffff88808cb36100 (0xffff88808cb36100)
00000000eb36eac9: 0000000000000000 ...
0000000084ea5e52: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000e809d5dc: ffff888079bc7ff8 (0xffff888079bc7ff8)
00000000e50d63ae: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000d90e11b6: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000000a06acca: ffff88808cb36100 (0xffff88808cb36100)
000000006c7f5c3c: 0000000000000000 ...
00000000e157dcd0: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000009844d018: ffff888079bc8028 (0xffff888079bc8028)
Modules linked in:
CR2: 000000004125973f
---[ end trace 79fdcdc2ec39f3e0 ]---
RIP: 0010:tls_prots+0x1a8a/0x3520
Code: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 70 0d 0f 86 ff ff ff ff 40 5d <28> 86 ff ff ff
ff 60 6e 28 86 ff ff ff ff 00 de ed 85 ff ff ff ff
RSP: 0018:ffff888079bc7c10 EFLAGS: 00010082
RAX: ffff88808c5226c0 RBX: ffff88808cb36100 RCX: 1ffffffff116885c
RDX: 1ffff110118a44d8 RSI: 0000000041259740 RDI: ffff88808c523a70
RBP: ffff888079bc7c38 R08: ffff88808c5226c0 R09: ffffed1015d26c70
R10: ffffed1015d26c6f R11: ffff8880ae93637b R12: ffffffff860dd760
R13: ffffffff860dda75 R14: ffff888079bc7c08 R15: ffffffff8b1da9a0
FS: 00007fa341259740(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000004125973f CR3: 00000000907f6000 CR4: 00000000001406e0
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCH 1/2] net: stmmac: Fix possible deadlock when disabling EEE support
From: Jon Hunter @ 2019-06-26 10:23 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: netdev, linux-kernel, linux-tegra, Jon Hunter
When stmmac_eee_init() is called to disable EEE support, then the timer
for EEE support is stopped and we return from the function. Prior to
stopping the timer, a mutex was acquired but in this case it is never
released and so could cause a deadlock. Fix this by releasing the mutex
prior to returning from stmmax_eee_init() when stopping the EEE timer.
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b628c697cee9..6c6c6ec3c781 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -402,6 +402,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
netdev_dbg(priv->dev, "disable EEE\n");
del_timer_sync(&priv->eee_ctrl_timer);
stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ mutex_unlock(&priv->lock);
return false;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] net: stmmac: Fix crash observed if PHY does not support EEE
From: Jon Hunter @ 2019-06-26 10:23 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: netdev, linux-kernel, linux-tegra, Jon Hunter
In-Reply-To: <20190626102322.18821-1-jonathanh@nvidia.com>
If the PHY does not support EEE mode, then a crash is observed when the
ethernet interface is enabled. The crash occurs, because if the PHY does
not support EEE, then although the EEE timer is never configured, it is
still marked as enabled and so the stmmac ethernet driver is still
trying to update the timer by calling mod_timer(). This triggers a BUG()
in the mod_timer() because we are trying to update a timer when there is
no callback function set because timer_setup() was never called for this
timer.
The problem is caused because we return true from the function
stmmac_eee_init(), marking the EEE timer as enabled, even when we have
not configured the EEE timer. Fix this by ensuring that we return false
if the PHY does not support EEE and hence, 'eee_active' is not set.
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6c6c6ec3c781..8f5ebd51859e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -398,10 +398,12 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
mutex_lock(&priv->lock);
/* Check if it needs to be deactivated */
- if (!priv->eee_active && priv->eee_enabled) {
- netdev_dbg(priv->dev, "disable EEE\n");
- del_timer_sync(&priv->eee_ctrl_timer);
- stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ if (!priv->eee_active) {
+ if (priv->eee_enabled) {
+ netdev_dbg(priv->dev, "disable EEE\n");
+ del_timer_sync(&priv->eee_ctrl_timer);
+ stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ }
mutex_unlock(&priv->lock);
return false;
}
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox