* Re: [PATCH v2 2/2] dpaa_eth: add ethtool coalesce control
From: David Miller @ 2018-11-17 3:42 UTC (permalink / raw)
To: madalin.bucur
Cc: netdev, roy.pledge, linux-kernel, leoyang.li, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <1542126591-5114-3-git-send-email-madalin.bucur@nxp.com>
From: Madalin Bucur <madalin.bucur@nxp.com>
Date: Tue, 13 Nov 2018 18:29:51 +0200
> + for_each_cpu(cpu, cpus) {
> + portal = qman_get_affine_portal(cpu);
> + res = qman_portal_set_iperiod(portal, period);
> + if (res)
> + return res;
> + res = qman_dqrr_set_ithresh(portal, thresh);
> + if (res)
> + return res;
Nope, you can't do it like this.
If any intermediate change fails, you have to unwind all of the
changes made up until that point.
Which means you'll have to store the previous setting somewhere
and reinstall those saved values in the error path.
^ permalink raw reply
* Re: [PATCH v1] tg3: optionally get mac address from devicetree
From: David Miller @ 2018-11-17 3:37 UTC (permalink / raw)
To: thesven73
Cc: svendev, siva.kallam, prashant, mchan, linux-kernel, netdev, arnd
In-Reply-To: <20181113161508.7427-1-TheSven73@googlemail.com>
From: thesven73@gmail.com
Date: Tue, 13 Nov 2018 11:15:08 -0500
> If the tg3 has a device node, and that node contains a valid
> mac address property, use that as the tg3's mac address.
>
> This behaviour was previously only present on SPARC, using a
> conditional compile (#ifdef CONFIG_SPARC), presumably because
> at the time, devicetree nodes for pci devices only worked on
> SPARC. However, this has recently been made universal, see
> commit 98d9f30c820d ("pci/of: Match PCI devices to OF nodes dynamically")
>
> Devicetree example:
> (see Documentation/devicetree/bindings/pci/pci.txt)
>
> &pcie {
> host@0 {
> #address-cells = <3>;
> #size-cells = <2>;
> reg = <0 0 0 0 0>;
> bcm5778: bcm5778@0 {
> reg = <0 0 0 0 0>;
> mac-address = [CA 11 AB 1E 10 01];
> };
> };
> };
>
> Signed-off-by: Sven Van Asbroeck <svendev@arcx.com>
Please use eth_platform_get_mac_address() and you can delete all of the
SPARC specific code altogether.
^ permalink raw reply
* Re: [PATCH][net-next] net: hns3: fix spelling mistake "failded" -> "failed"
From: David Miller @ 2018-11-17 3:35 UTC (permalink / raw)
To: colin.king
Cc: yisen.zhuang, salil.mehta, lipeng321, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20181113115057.13237-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Tue, 13 Nov 2018 11:50:57 +0000
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix, the spelling of "failded" is incorrect in dev_err and
> dev_warn messages. Fix this.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: mvneta: Don't advertise 2.5G modes
From: David Miller @ 2018-11-17 3:23 UTC (permalink / raw)
To: maxime.chevallier
Cc: andrew, antoine.tenart, netdev, gregory.clement, linux-kernel,
linux, thomas.petazzoni, linux-arm-kernel
In-Reply-To: <20181109081733.24458-1-maxime.chevallier@bootlin.com>
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Fri, 9 Nov 2018 09:17:33 +0100
> Using 2.5G speed relies on the SerDes lanes being configured
> accordingly. The lanes have to be reconfigured to switch between
> 1G and 2.5G, and for now only the bootloader does this configuration.
>
> In the case we add a Comphy driver to handle switching the lanes
> dynamically, it's better for now to stick with supporting only 1G and
> add advertisement for 2.5G once we really are capable of handling both
> speeds without problem.
>
> Since the interface mode is initialy taken from the DT, we want to make
> sure that adding comphy support won't break boards that don't update
> their dtb.
>
> Fixes: da58a931f248 ("net: mvneta: Add support for 2500Mbps SGMII")
> Reported-by: Andrew Lunn <andrew@lunn.ch>
> Reported-by: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Applied.
^ permalink raw reply
* [PATCH net v3] net: bridge: fix vlan stats use-after-free on destruction
From: Nikolay Aleksandrov @ 2018-11-16 16:50 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, bridge, syzkaller-bugs, Nikolay Aleksandrov
In-Reply-To: <20181114172703.5795-1-nikolay@cumulusnetworks.com>
Syzbot reported a use-after-free of the global vlan context on port vlan
destruction. When I added per-port vlan stats I missed the fact that the
global vlan context can be freed before the per-port vlan rcu callback.
There're a few different ways to deal with this, I've chosen to add a
new private flag that is set only when per-port stats are allocated so
we can directly check it on destruction without dereferencing the global
context at all. The new field in net_bridge_vlan uses a hole.
v2: cosmetic change, move the check to br_process_vlan_info where the
other checks are done
v3: add change log in the patch, add private (in-kernel only) flags in a
hole in net_bridge_vlan struct and use that instead of mixing
user-space flags with private flags
Fixes: 9163a0fc1f0c ("net: bridge: add support for per-port vlan stats")
Reported-by: syzbot+04681da557a0e49a52e5@syzkaller.appspotmail.com
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
After the jet lag mostly passed it has all come together in a cleaner
and less future error-prone way. :)
net/bridge/br_private.h | 7 +++++++
net/bridge/br_vlan.c | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 2920e06a5403..04c19a37e500 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -102,12 +102,18 @@ struct br_tunnel_info {
struct metadata_dst *tunnel_dst;
};
+/* private vlan flags */
+enum {
+ BR_VLFLAG_PER_PORT_STATS = BIT(0),
+};
+
/**
* struct net_bridge_vlan - per-vlan entry
*
* @vnode: rhashtable member
* @vid: VLAN id
* @flags: bridge vlan flags
+ * @priv_flags: private (in-kernel) bridge vlan flags
* @stats: per-cpu VLAN statistics
* @br: if MASTER flag set, this points to a bridge struct
* @port: if MASTER flag unset, this points to a port struct
@@ -127,6 +133,7 @@ struct net_bridge_vlan {
struct rhash_head tnode;
u16 vid;
u16 flags;
+ u16 priv_flags;
struct br_vlan_stats __percpu *stats;
union {
struct net_bridge *br;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 8c9297a01947..e84be08b8285 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -197,7 +197,7 @@ static void nbp_vlan_rcu_free(struct rcu_head *rcu)
v = container_of(rcu, struct net_bridge_vlan, rcu);
WARN_ON(br_vlan_is_master(v));
/* if we had per-port stats configured then free them here */
- if (v->brvlan->stats != v->stats)
+ if (v->priv_flags & BR_VLFLAG_PER_PORT_STATS)
free_percpu(v->stats);
v->stats = NULL;
kfree(v);
@@ -264,6 +264,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
err = -ENOMEM;
goto out_filt;
}
+ v->priv_flags |= BR_VLFLAG_PER_PORT_STATS;
} else {
v->stats = masterv->stats;
}
--
2.17.2
^ permalink raw reply related
* Re: [RFC v1 2/3] vxlan: add support for underlay in non-default VRF
From: David Ahern @ 2018-11-16 16:31 UTC (permalink / raw)
To: Alexis Bauvin, roopa, nicolas.dichtel; +Cc: netdev, akherbouche
In-Reply-To: <D9A396BA-D092-4302-B94A-EC12885E236A@scaleway.com>
On 11/16/18 2:41 AM, Alexis Bauvin wrote:
> The case I am trying to cover here is the user creating a VXLAN device with eth0
> as its lower device (ip link add vxlan0 type vxlan ... dev eth0), thus ignoring
> the fact that it should be br0 (the actual L3 interface). In this case, the only
> information available from the module's point of view is eth0. I may be wrong,
> but eth0 is indirectly "part" of vrf-blue (even if it is only L2), as packets
> flowing in from it would land in vrf-blue if L3.
for routing lookups, yes.
>
> As for the device stacking, I am only interested in the VXLAN underlay: the
> VXLAN device itself could be in a specific VRF or not, it should not influence
> its underlay.
>
> +----------+ +---------+
> | | | |
> | vrf-blue | | vrf-red |
> | | | |
> +----+-----+ +----+----+
> | |
> | |
> +----+-----+ +----+----+
> | | | |
> | br-blue | | br-red |
> | | | |
> +----+-----+ +---+-+---+
> | | |
> | +-----+ +-----+
> | | |
> +----+-----+ +------+----+ +----+----+
> | | lower device | | | |
> | eth0 | <- - - - - - - | vxlan-red | | tap-red | (... more taps)
> | | | | | |
> +----------+ +-----------+ +---------+
>
>
> While I don't see any use case for having a bridged uplink when using VXLAN,
> someone may and would find a different behavior depending on the lower device.
> In the above example, vxlan-red's lower device should be br-blue, but a user
> would expect the underlay VRF (vrf-blue) to still be taken into account if eth0
> was used as the lower device.
>
> A different approach would be to check if the lower device is a bridge. If not,
> fetch a potential master bridge. Then, with this L3/router interface, we fetch
> the l3mdev with l3mdev_master_ifindex_by_index (if any).
ok. got it. Add the above diagram to the commit message to document the
use case.
>
>>
>>> This is because the underlying l3mdev_master_dev_rcu function fetches the master
>>> (br0 in this case), checks whether it is an l3mdev (which it is not), and
>>> returns its index if so.
>>>
>>> So if using l3mdev_master_dev_rcu, using eth0 as a lower device will still bind
>>> to no specific device, thus in the default VRF.
>>>
>>> Maybe I should have patched l3mdev_master_dev_rcu to do a recursive resolution
>>> (as vxlan_get_l3mdev does), but I don’t know the impact of such a change.
>>
>> no, that is definitely the wrong the approach.
>
> Ok! What is the best approach in your opinion?
>
Add the new function to l3mdev.c. The name should be consistent with the
others -- so something like l3mdev_master_upper_by_index (l3mdev for the
namespace, you are passing an index and wanting the master device but in
this case want to walk upper devices).
Also, annotate with expected locking.
^ permalink raw reply
* Re: [PATCH net-next v2] geneve: Use empty braces for addr6 initializer
From: Joe Perches @ 2018-11-17 2:33 UTC (permalink / raw)
To: Nathan Chancellor, Stefano Brivio
Cc: David S. Miller, Sabrina Dubroca, netdev, linux-kernel
In-Reply-To: <20181116153701.GA16184@flashbox>
On Fri, 2018-11-16 at 08:37 -0700, Nathan Chancellor wrote:
> On Fri, Nov 16, 2018 at 03:04:32PM +0100, Stefano Brivio wrote:
> > On Mon, 12 Nov 2018 23:11:47 -0700
> > Nathan Chancellor <natechancellor@gmail.com> wrote:
> >
> > > Clang warns:
> > >
> > > drivers/net/geneve.c:428:29: error: suggest braces around initialization
> > > of subobject [-Werror,-Wmissing-braces]
> > > struct in6_addr addr6 = { 0 };
> > > ^
> > > {}
> > >
> > > Most initializations of structs in the kernel seem to use this format.
[]
> > So { { { 0 } } } seems to be the safest option. We could go with static
> > but it looks even uglier to me.
> >
> > Joe, suggestions?
If this is really an issue, I don't know why the other uses
haven't been reported. Perhaps change the rest of them too?
$ git grep -P "struct\s+in6_addr\s+\w+\s*=\s*{\s*}"
include/net/ip6_fib.h: struct in6_addr in6_zero = {};
include/trace/events/fib6.h: struct in6_addr in6_zero = {};
net/6lowpan/iphc.c: struct in6_addr network_pfx = {};
net/6lowpan/iphc.c: struct in6_addr tmp = {};
net/6lowpan/iphc.c: struct in6_addr tmp = {};
net/6lowpan/iphc.c: struct in6_addr tmp = {};
net/6lowpan/iphc.c: struct in6_addr tmp = {};
^ permalink raw reply
* [PATCH net] ip_tunnel: don't force DF when MTU is locked
From: Sabrina Dubroca @ 2018-11-16 15:58 UTC (permalink / raw)
To: netdev; +Cc: sbrivio, Sabrina Dubroca
The various types of tunnels running over IPv4 can ask to set the DF
bit to do PMTU discovery. However, PMTU discovery is subject to the
threshold set by the net.ipv4.route.min_pmtu sysctl, and is also
disabled on routes with "mtu lock". In those cases, we shouldn't set
the DF bit.
This patch makes setting the DF bit conditional on the route's MTU
locking state.
This issue seems to be older than git history.
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv4/ip_tunnel_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index f45b96d715f0..c857ec6b9784 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -80,7 +80,7 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
iph->version = 4;
iph->ihl = sizeof(struct iphdr) >> 2;
- iph->frag_off = df;
+ iph->frag_off = ip_mtu_locked(&rt->dst) ? 0 : df;
iph->protocol = proto;
iph->tos = tos;
iph->daddr = dst;
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 2/2] net: bcmgenet: abort suspend on error
From: Doug Berger @ 2018-11-17 2:00 UTC (permalink / raw)
To: David S. Miller
Cc: Florian Fainelli, netdev, linux-kernel, YueHaibing, Doug Berger
In-Reply-To: <1542420022-30251-1-git-send-email-opendmb@gmail.com>
If an error occurs during suspension of the driver the driver should
restore the hardware configuration and return an error to force the
system to resume.
Fixes: 0db55093b566 ("net: bcmgenet: return correct value 'ret' from bcmgenet_power_down")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 +++
drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 01c8499fd87f..983245c0867c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3717,6 +3717,9 @@ static int bcmgenet_suspend(struct device *d)
/* Turn off the clocks */
clk_disable_unprepare(priv->clk);
+ if (ret)
+ bcmgenet_resume(d);
+
return ret;
}
#endif /* CONFIG_PM_SLEEP */
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
index 2fbd027f0148..b3596e0ee47b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
@@ -186,9 +186,15 @@ void bcmgenet_wol_power_up_cfg(struct bcmgenet_priv *priv,
}
reg = bcmgenet_umac_readl(priv, UMAC_MPD_CTRL);
+ if (!(reg & MPD_EN))
+ return; /* already powered up so skip the rest */
reg &= ~MPD_EN;
bcmgenet_umac_writel(priv, reg, UMAC_MPD_CTRL);
+ reg = bcmgenet_hfb_reg_readl(priv, HFB_CTRL);
+ reg &= ~(RBUF_HFB_EN | RBUF_ACPI_EN);
+ bcmgenet_hfb_reg_writel(priv, reg, HFB_CTRL);
+
/* Disable CRC Forward */
reg = bcmgenet_umac_readl(priv, UMAC_CMD);
reg &= ~CMD_CRC_FWD;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next] net: align gnet_stats_basic_cpu struct
From: Eric Dumazet @ 2018-11-16 15:43 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
This structure is small (12 or 16 bytes depending on 64bit
or 32bit kernels), but we do not want it spanning two cache lines.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/gen_stats.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index 946bd53a9f81dff5946579514360a9e5eaf3489b..ca23860adbb956fcfff3605068fdedf59073ce1a 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -10,7 +10,7 @@
struct gnet_stats_basic_cpu {
struct gnet_stats_basic_packed bstats;
struct u64_stats_sync syncp;
-};
+} __aligned(2 * sizeof(u64));
struct net_rate_estimator;
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply related
* Re: [PATCH] bpf: Remove unused variable in nsim_bpf
From: Alexei Starovoitov @ 2018-11-17 1:52 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Alexei Starovoitov, Daniel Borkmann, Quentin Monnet,
Jakub Kicinski, netdev, linux-kernel
In-Reply-To: <20181112221042.11160-1-natechancellor@gmail.com>
On Mon, Nov 12, 2018 at 03:10:42PM -0700, Nathan Chancellor wrote:
> Clang warns:
>
> drivers/net/netdevsim/bpf.c:557:30: error: unused variable 'state'
> [-Werror,-Wunused-variable]
> struct nsim_bpf_bound_prog *state;
> ^
> 1 error generated.
>
> The declaration should have been removed in commit b07ade27e933 ("bpf:
> pass translate() as a callback and remove its ndo_bpf subcommand").
>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Applied, Thanks
^ permalink raw reply
* [PATCH net-next] net: align pcpu_sw_netstats and pcpu_lstats structs
From: Eric Dumazet @ 2018-11-16 15:24 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
Do not risk spanning these small structures on two cache lines,
it is absolutely not worth it.
For 32bit arches, the hint might not be enough, but we do not
really care anymore.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 917ae7b6263e4686ac7af4d16445f4e996001ea6..086e64d885971ff04f186d488975b3305a0fbb1d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2389,13 +2389,13 @@ struct pcpu_sw_netstats {
u64 tx_packets;
u64 tx_bytes;
struct u64_stats_sync syncp;
-};
+} __aligned(4 * sizeof(u64));
struct pcpu_lstats {
u64 packets;
u64 bytes;
struct u64_stats_sync syncp;
-};
+} __aligned(2 * sizeof(u64));
#define __netdev_alloc_pcpu_stats(type, gfp) \
({ \
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply related
* [PATCH net-next v3] geneve: Initialize addr6 with memset
From: Nathan Chancellor @ 2018-11-17 1:36 UTC (permalink / raw)
To: David S. Miller
Cc: Stefano Brivio, Sabrina Dubroca, netdev, linux-kernel,
Nathan Chancellor
In-Reply-To: <20181113061146.12182-1-natechancellor@gmail.com>
Clang warns:
drivers/net/geneve.c:428:29: error: suggest braces around initialization
of subobject [-Werror,-Wmissing-braces]
struct in6_addr addr6 = { 0 };
^
{}
Rather than trying to appease the various compilers that support the
kernel, use memset, which is unambiguous.
Fixes: a07966447f39 ("geneve: ICMP error lookup handler")
Suggested-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2:
* Just remove the zero instead of adding more braces as it seems to
match the rest of the kernel (as suggested by Joe Perches).
v2 -> v3:
* Use memset as suggested by David Miller.
drivers/net/geneve.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 7c53e06b31c3..58bbba8582b0 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -425,7 +425,9 @@ static int geneve_udp_encap_err_lookup(struct sock *sk, struct sk_buff *skb)
#if IS_ENABLED(CONFIG_IPV6)
if (geneve_get_sk_family(gs) == AF_INET6) {
struct ipv6hdr *ip6h = ipv6_hdr(skb);
- struct in6_addr addr6 = { 0 };
+ struct in6_addr addr6;
+
+ memset(&addr6, 0, sizeof(struct in6_addr));
if (!gs->collect_md) {
vni = geneve_hdr(skb)->vni;
--
2.19.1
^ permalink raw reply related
* Re: [PATCH net-next 7/8] net: eth: altera: tse: add msgdma prefetcher
From: Thor Thayer @ 2018-11-16 15:20 UTC (permalink / raw)
To: Dalon Westergreen, netdev, dinguyen; +Cc: Dalon Westergreen
In-Reply-To: <20181115005047.28464-8-dwesterg@gmail.com>
Hi Dalon,
Just a few comments/questions.
On 11/14/18 6:50 PM, Dalon Westergreen wrote:
> From: Dalon Westergreen <dalon.westergreen@intel.com>
>
> Add support for the mSGDMA prefetcher. The prefetcher adds support
> for a linked list of descriptors in system memory. The prefetcher
> feeds these to the mSGDMA dispatcher.
>
> The prefetcher is configured to poll for the next descriptor in the
> list to be owned by hardware, then pass the descriptor to the
> dispatcher. It will then poll the next descriptor until it is
> owned by hardware.
>
> The dispatcher responses are written back to the appropriate
> descriptor, and the owned by hardware bit is cleared.
>
> The driver sets up a linked list twice the tx and rx ring sizes,
> with the last descriptor pointing back to the first. This ensures
> that the ring of descriptors will always have inactive descriptors
> preventing the prefetcher from looping over and reusing descriptors
> inappropriately. The prefetcher will continuously loop over these
> descriptors. The driver modifies descriptors as required to update
> the skb address and length as well as the owned by hardware bit.
>
> In addition to the above, the mSGDMA prefetcher can be used to
> handle rx and tx timestamps coming from the ethernet ip. These
> can be included in the prefetcher response in the descriptor.
>
> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> ---
> drivers/net/ethernet/altera/Makefile | 2 +-
> .../altera/altera_msgdma_prefetcher.c | 433 ++++++++++++++++++
> .../altera/altera_msgdma_prefetcher.h | 30 ++
> .../altera/altera_msgdmahw_prefetcher.h | 87 ++++
> drivers/net/ethernet/altera/altera_tse.h | 14 +
> drivers/net/ethernet/altera/altera_tse_main.c | 51 +++
> 6 files changed, 616 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/altera/altera_msgdma_prefetcher.c
> create mode 100644 drivers/net/ethernet/altera/altera_msgdma_prefetcher.h
> create mode 100644 drivers/net/ethernet/altera/altera_msgdmahw_prefetcher.h
>
> diff --git a/drivers/net/ethernet/altera/Makefile b/drivers/net/ethernet/altera/Makefile
> index ad80be42fa26..73b32876f126 100644
> --- a/drivers/net/ethernet/altera/Makefile
> +++ b/drivers/net/ethernet/altera/Makefile
> @@ -5,4 +5,4 @@
> obj-$(CONFIG_ALTERA_TSE) += altera_tse.o
> altera_tse-objs := altera_tse_main.o altera_tse_ethtool.o \
> altera_msgdma.o altera_sgdma.o altera_utils.o \
> - altera_ptp.o
> + altera_ptp.o altera_msgdma_prefetcher.o
> diff --git a/drivers/net/ethernet/altera/altera_msgdma_prefetcher.c b/drivers/net/ethernet/altera/altera_msgdma_prefetcher.c
> new file mode 100644
> index 000000000000..55b475e9e15b
> --- /dev/null
> +++ b/drivers/net/ethernet/altera/altera_msgdma_prefetcher.c
> @@ -0,0 +1,433 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* MSGDMA Prefetcher driver for Altera ethernet devices
> + *
> + * Copyright (C) 2018 Intel Corporation. All rights reserved.
> + * Author(s):
> + * Dalon Westergreen <dalon.westergreen@intel.com>
> + */
> +
> +#include <linux/list.h>
> +#include <linux/netdevice.h>
> +#include <linux/net_tstamp.h>
> +#include "altera_utils.h"
> +#include "altera_tse.h"
> +#include "altera_msgdma.h"
> +#include "altera_msgdmahw.h"
> +#include "altera_msgdma_prefetcher.h"
> +#include "altera_msgdmahw_prefetcher.h"
These could be alphabetized - tse and utils at the end.
> +
> +int msgdma_pref_initialize(struct altera_tse_private *priv)
> +{
> + int i;
> + struct msgdma_pref_extended_desc *rx_descs;
> + struct msgdma_pref_extended_desc *tx_descs;
> + dma_addr_t rx_descsphys;
> + dma_addr_t tx_descsphys;
> + u32 rx_ring_size;
> + u32 tx_ring_size;
> +
> + priv->pref_rxdescphys = (dma_addr_t)0;
> + priv->pref_txdescphys = (dma_addr_t)0;
> +
> + /* we need to allocate more pref descriptors than ringsize, for now
> + * just double ringsize
> + */
> + rx_ring_size = priv->rx_ring_size * 2;
> + tx_ring_size = priv->tx_ring_size * 2;
> +
> + /* The prefetcher requires the descriptors to be aligned to the
> + * descriptor read/write master's data width which worst case is
> + * 512 bits. Currently we DO NOT CHECK THIS and only support 32-bit
> + * prefetcher masters.
> + */
> +
> + /* allocate memory for rx descriptors */
> + priv->pref_rxdesc =
> + dma_zalloc_coherent(priv->device,
> + sizeof(struct msgdma_pref_extended_desc)
> + * rx_ring_size,
> + &priv->pref_rxdescphys, GFP_KERNEL);
> +
> + if (!priv->pref_rxdesc)
> + goto err_rx;
> +
> + /* allocate memory for tx descriptors */
> + priv->pref_txdesc =
> + dma_zalloc_coherent(priv->device,
> + sizeof(struct msgdma_pref_extended_desc)
> + * tx_ring_size,
> + &priv->pref_txdescphys, GFP_KERNEL);
> +
> + if (!priv->pref_txdesc)
> + goto err_tx;
> +
> + /* setup base descriptor ring for tx & rx */
> + rx_descs = (struct msgdma_pref_extended_desc *)priv->pref_rxdesc;
> + tx_descs = (struct msgdma_pref_extended_desc *)priv->pref_txdesc;
> + tx_descsphys = priv->pref_txdescphys;
> + rx_descsphys = priv->pref_rxdescphys;
> +
> + /* setup RX descriptors */
> + priv->pref_rx_prod = 0;
> + for (i = 0; i < rx_ring_size; i++) {
> + rx_descsphys = priv->pref_rxdescphys +
> + (((i + 1) % rx_ring_size) *
> + sizeof(struct msgdma_pref_extended_desc));
> + rx_descs[i].next_desc_lo = lower_32_bits(rx_descsphys);
> + rx_descs[i].next_desc_hi = upper_32_bits(rx_descsphys);
> + rx_descs[i].stride = MSGDMA_DESC_RX_STRIDE;
> + /* burst set to 0 so it defaults to max configured */
> + /* set seq number to desc number */
> + rx_descs[i].burst_seq_num = i;
> + }
> +
> + /* setup TX descriptors */
> + for (i = 0; i < tx_ring_size; i++) {
> + tx_descsphys = priv->pref_txdescphys +
> + (((i + 1) % tx_ring_size) *
> + sizeof(struct msgdma_pref_extended_desc));
> + tx_descs[i].next_desc_lo = lower_32_bits(tx_descsphys);
> + tx_descs[i].next_desc_hi = upper_32_bits(tx_descsphys);
> + tx_descs[i].stride = MSGDMA_DESC_TX_STRIDE;
> + /* burst set to 0 so it defaults to max configured */
> + /* set seq number to desc number */
> + tx_descs[i].burst_seq_num = i;
> + }
> +
> + if (netif_msg_ifup(priv))
> + netdev_info(priv->dev, "%s: RX Desc mem at 0x%x\n", __func__,
> + priv->pref_rxdescphys);
> +
> + if (netif_msg_ifup(priv))
> + netdev_info(priv->dev, "%s: TX Desc mem at 0x%x\n", __func__,
> + priv->pref_txdescphys);
> +
> + return 0;
> +
> +err_tx:
> + dma_free_coherent(priv->device,
> + sizeof(struct msgdma_pref_extended_desc)
> + * rx_ring_size,
> + priv->pref_rxdesc, priv->pref_rxdescphys);
> +err_rx:
> + return -ENOMEM;
> +}
> +
> +void msgdma_pref_uninitialize(struct altera_tse_private *priv)
> +{
> + if (priv->pref_rxdesc)
> + dma_free_coherent(priv->device,
> + sizeof(struct msgdma_pref_extended_desc)
> + * priv->rx_ring_size * 2,
> + priv->pref_rxdesc, priv->pref_rxdescphys);
> +
> + if (priv->pref_txdesc)
> + dma_free_coherent(priv->device,
> + sizeof(struct msgdma_pref_extended_desc)
> + * priv->rx_ring_size * 2,
> + priv->pref_txdesc, priv->pref_txdescphys);
Why does this have the ring_size*2 but the error path in
msgdma_pref_initialize() above (err_tx path) doesn't?
> +}
> +
> +void msgdma_pref_enable_txirq(struct altera_tse_private *priv)
> +{
> + tse_set_bit(priv->tx_pref_csr, msgdma_pref_csroffs(control),
> + MSGDMA_PREF_CTL_GLOBAL_INTR);
> +}
> +
<snip>
> +
> +void msgdma_pref_reset(struct altera_tse_private *priv)
> +{
> + int counter;
> +
> + /* turn off polling */
> + tse_clear_bit(priv->rx_pref_csr, msgdma_pref_csroffs(control),
> + MSGDMA_PREF_CTL_DESC_POLL_EN);
> + tse_clear_bit(priv->tx_pref_csr, msgdma_pref_csroffs(control),
> + MSGDMA_PREF_CTL_DESC_POLL_EN);
> +
> + /* Reset the RX Prefetcher */
> + csrwr32(MSGDMA_PREF_STAT_IRQ, priv->rx_pref_csr,
> + msgdma_pref_csroffs(status));
> + csrwr32(MSGDMA_PREF_CTL_RESET, priv->rx_pref_csr,
> + msgdma_pref_csroffs(control));
> +
> + counter = 0;
> + while (counter++ < ALTERA_TSE_SW_RESET_WATCHDOG_CNTR) {
> + if (tse_bit_is_clear(priv->rx_pref_csr,
> + msgdma_pref_csroffs(control),
> + MSGDMA_PREF_CTL_RESET))
> + break;
> + udelay(1);
> + }
> +
> + if (counter >= ALTERA_TSE_SW_RESET_WATCHDOG_CNTR)
> + netif_warn(priv, drv, priv->dev,
> + "TSE Rx Prefetcher reset bit never cleared!\n");
> +
I take it there are no negative consequences for the reset bit not
clearing? Would it be useful to return an error?
> + /* clear all status bits */
> + csrwr32(MSGDMA_PREF_STAT_IRQ, priv->tx_pref_csr,
> + msgdma_pref_csroffs(status));
> +
This looks the same as below. Are they the same or did I miss something?
> + /* Reset the TX Prefetcher */
> + csrwr32(MSGDMA_PREF_STAT_IRQ, priv->tx_pref_csr,
> + msgdma_pref_csroffs(status));
> + csrwr32(MSGDMA_PREF_CTL_RESET, priv->tx_pref_csr,
> + msgdma_pref_csroffs(control));
> +
> + counter = 0;
> + while (counter++ < ALTERA_TSE_SW_RESET_WATCHDOG_CNTR) {
> + if (tse_bit_is_clear(priv->tx_pref_csr,
> + msgdma_pref_csroffs(control),
> + MSGDMA_PREF_CTL_RESET))
> + break;
> + udelay(1);
> + }
> +
> + if (counter >= ALTERA_TSE_SW_RESET_WATCHDOG_CNTR)
> + netif_warn(priv, drv, priv->dev,
> + "TSE Tx Prefetcher reset bit never cleared!\n");
> +
Same as above.
> + /* clear all status bits */
> + csrwr32(MSGDMA_PREF_STAT_IRQ, priv->tx_pref_csr,
> + msgdma_pref_csroffs(status));
> +
> + /* Reset mSGDMA dispatchers*/
> + msgdma_reset(priv);
> +}
> +
<snip>
> +
> +/* Add MSGDMA Prefetcher Descriptor to descriptor list
> + * -> This should never be called when a descriptor isn't available
> + */
> +void msgdma_pref_add_rx_desc(struct altera_tse_private *priv,
> + struct tse_buffer *rxbuffer)
> +{
> + struct msgdma_pref_extended_desc *rx_descs = priv->pref_rxdesc;
> + u32 desc_entry = priv->pref_rx_prod % (priv->rx_ring_size * 2);
> +
> + /* write descriptor entries */
> + rx_descs[desc_entry].len = priv->rx_dma_buf_sz;
> + rx_descs[desc_entry].write_addr_lo = lower_32_bits(rxbuffer->dma_addr);
> + rx_descs[desc_entry].write_addr_hi = upper_32_bits(rxbuffer->dma_addr);
> +
> + /* set the control bits and set owned by hw */
> + rx_descs[desc_entry].desc_control = (MSGDMA_DESC_CTL_END_ON_EOP
> + | MSGDMA_DESC_CTL_END_ON_LEN
> + | MSGDMA_DESC_CTL_TR_COMP_IRQ
> + | MSGDMA_DESC_CTL_EARLY_IRQ
> + | MSGDMA_DESC_CTL_TR_ERR_IRQ
> + | MSGDMA_DESC_CTL_GO
> + | MSGDMA_PREF_DESC_CTL_OWNED_BY_HW);
> +
> + /* we need to keep a separate one for rx as RX_DESCRIPTORS are
> + * pre-configured at startup
> + */
> + priv->pref_rx_prod++;
Can you explain more in the comment? What is "one"?
> +
> + if (netif_msg_rx_status(priv)) {
> + netdev_info(priv->dev, "%s: desc: %d buf: %d control 0x%x\n",
> + __func__, desc_entry,
> + priv->rx_prod % priv->rx_ring_size,
> + priv->pref_rxdesc[desc_entry].desc_control);
> + }
> +}
> +
<snip>
Thanks for the patches!
^ permalink raw reply
* Re: [PATCH net] sctp: not allow to set asoc prsctp_enable by sockopt
From: Neil Horman @ 2018-11-16 15:12 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: lucien xin, netdev, linux-sctp, davem
In-Reply-To: <20181115234101.GC31918@localhost.localdomain>
On Thu, Nov 15, 2018 at 09:41:01PM -0200, Marcelo Ricardo Leitner wrote:
> [ re-sending, without html this time ]
>
> On Thu, Nov 15, 2018, 15:26 Neil Horman <nhorman@tuxdriver.com wrote:
>
> > On Thu, Nov 15, 2018 at 08:25:36PM -0200, Marcelo Ricardo Leitner wrote:
> > > On Thu, Nov 15, 2018 at 04:43:10PM -0500, Neil Horman wrote:
> > > > On Thu, Nov 15, 2018 at 03:22:21PM -0200, Marcelo Ricardo Leitner
> > wrote:
> > > > > On Thu, Nov 15, 2018 at 07:14:28PM +0800, Xin Long wrote:
> > > > > > As rfc7496#section4.5 says about SCTP_PR_SUPPORTED:
> > > > > >
> > > > > > This socket option allows the enabling or disabling of the
> > > > > > negotiation of PR-SCTP support for future associations. For
> > existing
> > > > > > associations, it allows one to query whether or not PR-SCTP
> > support
> > > > > > was negotiated on a particular association.
> > > > > >
> > > > > > It means only sctp sock's prsctp_enable can be set.
> > > > > >
> > > > > > Note that for the limitation of SCTP_{CURRENT|ALL}_ASSOC, we will
> > > > > > add it when introducing SCTP_{FUTURE|CURRENT|ALL}_ASSOC for linux
> > > > > > sctp in another patchset.
> > > > > >
> > > > > > Fixes: 28aa4c26fce2 ("sctp: add SCTP_PR_SUPPORTED on sctp sockopt")
> > > > > > Reported-by: Ying Xu <yinxu@redhat.com>
> > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > > ---
> > > > > > net/sctp/socket.c | 13 +++----------
> > > > > > 1 file changed, 3 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > > > > index 739f3e5..e9b8232 100644
> > > > > > --- a/net/sctp/socket.c
> > > > > > +++ b/net/sctp/socket.c
> > > > > > @@ -3940,7 +3940,6 @@ static int
> > sctp_setsockopt_pr_supported(struct sock *sk,
> > > > > > unsigned int optlen)
> > > > > > {
> > > > > > struct sctp_assoc_value params;
> > > > > > - struct sctp_association *asoc;
> > > > > > int retval = -EINVAL;
> > > > > >
> > > > > > if (optlen != sizeof(params))
> > > > > > @@ -3951,16 +3950,10 @@ static int
> > sctp_setsockopt_pr_supported(struct sock *sk,
> > > > > > goto out;
> > > > > > }
> > > > > >
> > > > > > - asoc = sctp_id2assoc(sk, params.assoc_id);
> > > > > > - if (asoc) {
> > > > > > - asoc->prsctp_enable = !!params.assoc_value;
> > > > > > - } else if (!params.assoc_id) {
> > > > > > - struct sctp_sock *sp = sctp_sk(sk);
> > > > > > -
> > > > > > - sp->ep->prsctp_enable = !!params.assoc_value;
> > > > > > - } else {
> > > > > > + if (sctp_style(sk, UDP) && sctp_id2assoc(sk,
> > params.assoc_id))
> > > > >
> > > > > This would allow using a non-existent assoc id on UDP-style sockets
> > to
> > > > > set it at the socket, which is not expected. It should be more like:
> > > > >
> > > > > + if (sctp_style(sk, UDP) && params.assoc_id)
> > > > How do you see that to be the case? sctp_id2assoc will return NULL if
> > an
> > > > association isn't found, so the use of sctp_id2assoc should work just
> > fine.
> > >
> > > Right, it will return NULL, and because of that it won't bail out as
> > > it should and will adjust the socket config instead.
> > >
> >
> > Oh, duh, you're absolutely right, NULL will evalutate to false there, and
> > skip
> > the conditional goto out;
> >
> > that said, It would make more sense to me to just change the sense of the
> > second
> > condition to !sctp_id2assoc(sk, params.assoc_id), so that we goto out if no
> > association is found. it still seems a
>
>
> That would break setting it on the socket without an assoc so far.
>
ok, yes, I see what xin is getting at now. The RFC indicates that the
setsockopt method for this socket option is meant to set the prsctp enabled
value on _future_ associations, implying that we should not operate at all on
already existing associations (i.e. we should ignore the assoc_id in the passed
in structure and only operate on the socket). That said, heres the entire text
of the RFC section:
4.5. Socket Option for Getting and Setting the PR-SCTP Support
(SCTP_PR_SUPPORTED)
This socket option allows the enabling or disabling of the
negotiation of PR-SCTP support for future associations. For existing
associations, it allows one to query whether or not PR-SCTP support
was negotiated on a particular association.
Whether or not PR-SCTP is enabled by default is implementation
specific.
This socket option uses IPPROTO_SCTP as its level and
SCTP_PR_SUPPORTED as its name. It can be used with getsockopt() and
setsockopt(). The socket option value uses the following structure
defined in [RFC6458]:
struct sctp_assoc_value {
sctp_assoc_t assoc_id;
uint32_t assoc_value;
};
assoc_id: This parameter is ignored for one-to-one style sockets.
For one-to-many style sockets, this parameter indicates upon which
association the user is performing an action. The special
sctp_assoc_t SCTP_FUTURE_ASSOC can also be used; it is an error to
use SCTP_{CURRENT|ALL}_ASSOC in assoc_id.
assoc_value: A non-zero value encodes the enabling of PR-SCTP,
whereas a value of 0 encodes the disabling of PR-SCTP.
sctp_opt_info() needs to be extended to support SCTP_PR_SUPPORTED
My read of this suggests that for setting the prsctp_enabled flag, we only need
a valid socket (the presence or lack of associations is irrelevant), its only
for the getsockopt method that we need to specify an assoc_id, as the getsockopt
method operates on associations, while the setsockopt method operates at the
socket level (to be inherited as association init).
Given that, I'd argue that we can skip the check entirely, and just assign
sctp_sock(sk)->prsctp_enabled = !!param.assoc_value
directly.
Neil
> bit dodgy to me to just check if
> > params.assoc_id is non-zero, as that will allow userspace to pass invalid
> > assoc
> > ids in and have those trigger pr support updates.
> >
>
> Quite the other way around, no? By only checking if associd is non zero and
> exiting due to it we are making sure the user cannot use invalid IDs.
>
>
> > Neil
> >
> >
> >
^ permalink raw reply
* Re: [PATCH net-next 2/2] net/sched: act_police: don't use spinlock in the data path
From: Eric Dumazet @ 2018-11-16 14:55 UTC (permalink / raw)
To: David Laight, 'Eric Dumazet', Davide Caratti,
Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller
Cc: netdev@vger.kernel.org
In-Reply-To: <a3ed82c1679e4bb5a8ab4233df38a03c@AcuMS.aculab.com>
On 11/16/2018 06:41 AM, David Laight wrote:
> From: Eric Dumazet
>> Sent: 16 November 2018 14:35
> ...
>> I suggest to use a single cache line with a dedicated spinlock and these three s64
>>
>> spinlock_t tcfp_lock ____cacheline_aligned_in_smp;
>> s64 ...
>> s64 ...
>> s64 ...
>
> Doesn't this do something really stupid when cache lines are big.
> If the spinlock is 8 bytes you never want more than 32 byte alignment.
> If cache lines are 256 bytes you don't even need that.
We do want that, even if cache lines are 256 bytes, thank you.
>
> Also ISTR that the kmalloc() only guarantees 8 byte alignment on x86_64.
> So aligning structure members to larger offsets is rather pointless.
No it is not, we use these hints all the time.
Just double check and report a bug to mm teams if you disagree.
Please do not send feedback if you are not sure.
^ permalink raw reply
* [PATCH 4/5] Bluetooth: btusb: Collect the common Intel assignments together
From: Rajat Jain @ 2018-11-17 1:07 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
linux-bluetooth, linux-kernel, linux-usb, netdev
Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
sukumar.ghorai
In-Reply-To: <20181117010748.24347-1-rajatja@google.com>
The BTUSB_INTEL and BTUSB_INTEL_NEW have common functions & quirks are
assigned to hdev structure. Lets collect them together instead of
repeating them in different code branches.
Signed-off-by: Rajat Jain <rajatja@google.com>
---
drivers/bluetooth/btusb.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7439a7eb50ac..e8e148480c91 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3077,28 +3077,25 @@ static int btusb_probe(struct usb_interface *intf,
data->diag = usb_ifnum_to_if(data->udev, ifnum_base + 2);
}
#endif
+ if (id->driver_info & BTUSB_INTEL ||
+ id->driver_info & BTUSB_INTEL_NEW) {
- if (id->driver_info & BTUSB_INTEL) {
hdev->manufacturer = 2;
- hdev->setup = btusb_setup_intel;
- hdev->shutdown = btusb_shutdown_intel;
- hdev->set_diag = btintel_set_diag_mfg;
hdev->set_bdaddr = btintel_set_bdaddr;
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
- }
- if (id->driver_info & BTUSB_INTEL_NEW) {
- hdev->manufacturer = 2;
- hdev->send = btusb_send_frame_intel;
- hdev->setup = btusb_setup_intel_new;
- hdev->hw_error = btintel_hw_error;
- hdev->set_diag = btintel_set_diag;
- hdev->set_bdaddr = btintel_set_bdaddr;
- set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
- set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
- set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
+ if (id->driver_info & BTUSB_INTEL) {
+ hdev->setup = btusb_setup_intel;
+ hdev->shutdown = btusb_shutdown_intel;
+ hdev->set_diag = btintel_set_diag_mfg;
+ } else {
+ hdev->send = btusb_send_frame_intel;
+ hdev->setup = btusb_setup_intel_new;
+ hdev->hw_error = btintel_hw_error;
+ hdev->set_diag = btintel_set_diag;
+ }
}
if (id->driver_info & BTUSB_MARVELL)
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply related
* [PATCH 3/5] Bluetooth: Reset Bluetooth chip after multiple command timeouts
From: Rajat Jain @ 2018-11-17 1:07 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
linux-bluetooth, linux-kernel, linux-usb, netdev
Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
sukumar.ghorai
In-Reply-To: <20181117010748.24347-1-rajatja@google.com>
Add a quirk and a hook to allow the HCI core to reset the BT chip
if needed (after a number of timed out commands). Use that new hook to
initiate BT chip reset if the controller fails to respond to certain
number of commands (currently 5) including the HCI reset commands.
This is done based on a newly introduced quirk. This is done based
on some initial work by Intel.
Signed-off-by: Rajat Jain <rajatja@google.com>
---
include/net/bluetooth/hci.h | 8 ++++++++
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_core.c | 15 +++++++++++++--
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c36dc1e20556..af02fa5ffe54 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -192,6 +192,14 @@ enum {
*
*/
HCI_QUIRK_NON_PERSISTENT_SETUP,
+
+ /* When this quirk is set, hw_reset() would be run to reset the
+ * hardware, after a certain number of commands (currently 5)
+ * time out because the device fails to respond.
+ *
+ * This quirk should be set before hci_register_dev is called.
+ */
+ HCI_QUIRK_HW_RESET_ON_TIMEOUT,
};
/* HCI device flags */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e5ea633ea368..b86218304b80 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -313,6 +313,7 @@ struct hci_dev {
unsigned int acl_cnt;
unsigned int sco_cnt;
unsigned int le_cnt;
+ unsigned int timeout_cnt;
unsigned int acl_mtu;
unsigned int sco_mtu;
@@ -437,6 +438,7 @@ struct hci_dev {
int (*post_init)(struct hci_dev *hdev);
int (*set_diag)(struct hci_dev *hdev, bool enable);
int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
+ void (*hw_reset)(struct hci_dev *hdev);
};
#define HCI_PHY_HANDLE(handle) (handle & 0xff)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7352fe85674b..ab3a6a8b7ba6 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2569,13 +2569,24 @@ static void hci_cmd_timeout(struct work_struct *work)
struct hci_dev *hdev = container_of(work, struct hci_dev,
cmd_timer.work);
+ hdev->timeout_cnt++;
if (hdev->sent_cmd) {
struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
u16 opcode = __le16_to_cpu(sent->opcode);
- bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode);
+ bt_dev_err(hdev, "command 0x%4.4x tx timeout (cnt = %u)",
+ opcode, hdev->timeout_cnt);
} else {
- bt_dev_err(hdev, "command tx timeout");
+ bt_dev_err(hdev, "command tx timeout (cnt = %u)",
+ hdev->timeout_cnt);
+ }
+
+ if (test_bit(HCI_QUIRK_HW_RESET_ON_TIMEOUT, &hdev->quirks) &&
+ hdev->timeout_cnt >= 5) {
+ hdev->timeout_cnt = 0;
+ if (hdev->hw_reset)
+ hdev->hw_reset(hdev);
+ return;
}
atomic_set(&hdev->cmd_cnt, 1);
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply related
* [PATCH 2/5] usb: assign ACPI companions for embedded USB devices
From: Rajat Jain @ 2018-11-17 1:07 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
linux-bluetooth, linux-kernel, linux-usb, netdev
Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
sukumar.ghorai
In-Reply-To: <20181117010748.24347-1-rajatja@google.com>
From: Dmitry Torokhov <dtor@chromium.org>
USB devices permanently connected to USB ports may be described in ACPI
tables and share ACPI devices with ports they are connected to. See [1]
for details.
This will allow us to describe sideband resources for devices, such as,
for example, hard reset line for BT USB controllers.
[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects#acpi-namespace-hierarchy-and-adr-for-embedded-usb-devices
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Rajat Jain <rajatja@google.com> (changed how we get the usb_port)
---
drivers/usb/core/usb-acpi.c | 44 +++++++++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 8ff73c83e8e8..9043d7242d67 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -200,30 +200,56 @@ static struct acpi_device *
usb_acpi_find_companion_for_device(struct usb_device *udev)
{
struct acpi_device *adev;
+ struct usb_port *port_dev;
+ struct usb_hub *hub;
+
+ if (!udev->parent) {
+ /* root hub is only child (_ADR=0) under its parent, the HC */
+ adev = ACPI_COMPANION(udev->dev.parent);
+ return acpi_find_child_device(adev, 0, false);
+ }
- if (!udev->parent)
+ hub = usb_hub_to_struct_hub(udev->parent);
+ if (!hub)
return NULL;
- /* root hub is only child (_ADR=0) under its parent, the HC */
- adev = ACPI_COMPANION(udev->dev.parent);
- return acpi_find_child_device(adev, 0, false);
+ /*
+ * This is an embedded USB device connected to a port and such
+ * devices share port's ACPI companion.
+ */
+ port_dev = hub->ports[udev->portnum - 1];
+ return usb_acpi_get_companion_for_port(port_dev);
}
-
static struct acpi_device *usb_acpi_find_companion(struct device *dev)
{
/*
- * In the ACPI DSDT table, only usb root hub and usb ports are
- * acpi device nodes. The hierarchy like following.
+ * The USB hierarchy like following:
+ *
* Device (EHC1)
* Device (HUBN)
* Device (PR01)
* Device (PR11)
* Device (PR12)
+ * Device (FN12)
+ * Device (FN13)
* Device (PR13)
* ...
- * So all binding process is divided into two parts. binding
- * root hub and usb ports.
+ * where HUBN is root hub, and PRNN are USB ports and devices
+ * connected to them, and FNNN are individualk functions for
+ * connected composite USB devices. PRNN and FNNN may contain
+ * _CRS and other methods describing sideband resources for
+ * the connected device.
+ *
+ * On the kernel side both root hub and embedded USB devices are
+ * represented as instances of usb_device structure, and ports
+ * are represented as usb_port structures, so the whole process
+ * is split into 2 parts: finding companions for devices and
+ * finding companions for ports.
+ *
+ * Note that we do not handle individual functions of composite
+ * devices yet, for that we would need to assign companions to
+ * devices corresponding to USB interfaces.
*/
if (is_usb_device(dev))
return usb_acpi_find_companion_for_device(to_usb_device(dev));
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply related
* [PATCH 1/5] usb: split code locating ACPI companion into port and device
From: Rajat Jain @ 2018-11-17 1:07 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
linux-bluetooth, linux-kernel, linux-usb, netdev
Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
sukumar.ghorai
In-Reply-To: <20181117010748.24347-1-rajatja@google.com>
From: Dmitry Torokhov <dtor@chromium.org>
In preparation for handling embedded USB devices let's split
usb_acpi_find_companion() into usb_acpi_find_companion_for_device() and
usb_acpi_find_companion_for_port().
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Rajat Jain <rajatja@google.com>
---
drivers/usb/core/usb-acpi.c | 133 +++++++++++++++++++-----------------
1 file changed, 72 insertions(+), 61 deletions(-)
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index e221861b3187..8ff73c83e8e8 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -139,12 +139,79 @@ static struct acpi_device *usb_acpi_find_port(struct acpi_device *parent,
return acpi_find_child_device(parent, raw, false);
}
-static struct acpi_device *usb_acpi_find_companion(struct device *dev)
+static struct acpi_device *
+usb_acpi_get_companion_for_port(struct usb_port *port_dev)
{
struct usb_device *udev;
struct acpi_device *adev;
acpi_handle *parent_handle;
+ int port1;
+
+ /* Get the struct usb_device point of port's hub */
+ udev = to_usb_device(port_dev->dev.parent->parent);
+
+ /*
+ * The root hub ports' parent is the root hub. The non-root-hub
+ * ports' parent is the parent hub port which the hub is
+ * connected to.
+ */
+ if (!udev->parent) {
+ adev = ACPI_COMPANION(&udev->dev);
+ port1 = usb_hcd_find_raw_port_number(bus_to_hcd(udev->bus),
+ port_dev->portnum);
+ } else {
+ parent_handle = usb_get_hub_port_acpi_handle(udev->parent,
+ udev->portnum);
+ if (!parent_handle)
+ return NULL;
+
+ acpi_bus_get_device(parent_handle, &adev);
+ port1 = port_dev->portnum;
+ }
+
+ return usb_acpi_find_port(adev, port1);
+}
+
+static struct acpi_device *
+usb_acpi_find_companion_for_port(struct usb_port *port_dev)
+{
+ struct acpi_device *adev;
+ struct acpi_pld_info *pld;
+ acpi_handle *handle;
+ acpi_status status;
+
+ adev = usb_acpi_get_companion_for_port(port_dev);
+ if (!adev)
+ return NULL;
+
+ handle = adev->handle;
+ status = acpi_get_physical_device_location(handle, &pld);
+ if (!ACPI_FAILURE(status) && pld) {
+ port_dev->location = USB_ACPI_LOCATION_VALID
+ | pld->group_token << 8 | pld->group_position;
+ port_dev->connect_type = usb_acpi_get_connect_type(handle, pld);
+ ACPI_FREE(pld);
+ }
+ return adev;
+}
+
+static struct acpi_device *
+usb_acpi_find_companion_for_device(struct usb_device *udev)
+{
+ struct acpi_device *adev;
+
+ if (!udev->parent)
+ return NULL;
+
+ /* root hub is only child (_ADR=0) under its parent, the HC */
+ adev = ACPI_COMPANION(udev->dev.parent);
+ return acpi_find_child_device(adev, 0, false);
+}
+
+
+static struct acpi_device *usb_acpi_find_companion(struct device *dev)
+{
/*
* In the ACPI DSDT table, only usb root hub and usb ports are
* acpi device nodes. The hierarchy like following.
@@ -158,66 +225,10 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
* So all binding process is divided into two parts. binding
* root hub and usb ports.
*/
- if (is_usb_device(dev)) {
- udev = to_usb_device(dev);
- if (udev->parent)
- return NULL;
-
- /* root hub is only child (_ADR=0) under its parent, the HC */
- adev = ACPI_COMPANION(dev->parent);
- return acpi_find_child_device(adev, 0, false);
- } else if (is_usb_port(dev)) {
- struct usb_port *port_dev = to_usb_port(dev);
- int port1 = port_dev->portnum;
- struct acpi_pld_info *pld;
- acpi_handle *handle;
- acpi_status status;
-
- /* Get the struct usb_device point of port's hub */
- udev = to_usb_device(dev->parent->parent);
-
- /*
- * The root hub ports' parent is the root hub. The non-root-hub
- * ports' parent is the parent hub port which the hub is
- * connected to.
- */
- if (!udev->parent) {
- struct usb_hcd *hcd = bus_to_hcd(udev->bus);
- int raw;
-
- raw = usb_hcd_find_raw_port_number(hcd, port1);
-
- adev = usb_acpi_find_port(ACPI_COMPANION(&udev->dev),
- raw);
-
- if (!adev)
- return NULL;
- } else {
- parent_handle =
- usb_get_hub_port_acpi_handle(udev->parent,
- udev->portnum);
- if (!parent_handle)
- return NULL;
-
- acpi_bus_get_device(parent_handle, &adev);
-
- adev = usb_acpi_find_port(adev, port1);
-
- if (!adev)
- return NULL;
- }
- handle = adev->handle;
- status = acpi_get_physical_device_location(handle, &pld);
- if (ACPI_FAILURE(status) || !pld)
- return adev;
-
- port_dev->location = USB_ACPI_LOCATION_VALID
- | pld->group_token << 8 | pld->group_position;
- port_dev->connect_type = usb_acpi_get_connect_type(handle, pld);
- ACPI_FREE(pld);
-
- return adev;
- }
+ if (is_usb_device(dev))
+ return usb_acpi_find_companion_for_device(to_usb_device(dev));
+ else if (is_usb_port(dev))
+ return usb_acpi_find_companion_for_port(to_usb_port(dev));
return NULL;
}
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply related
* [PATCH 0/5] Reset Intel BT controller if it gets stuck
From: Rajat Jain @ 2018-11-17 1:07 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
linux-bluetooth, linux-kernel, linux-usb, netdev
Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
sukumar.ghorai
There can be error conditions within Intel BT firmware that can cause it
to get stuck, with the only way out being toggle the reset pin to the
device. (I do not have the details about the issues that lead to such
conditions, Intel folks copied here can elaborate if needed). Thus, this
is an effor to be able to toggle the reset line from the driver if it
detects such a situation. It makes few enhancements to the common
framework which I think may be useful for other unrelated problems.
Dmitry Torokhov (2):
usb: split code locating ACPI companion into port and device
usb: assign ACPI companions for embedded USB devices
(This basically allows ACPI nodes to be attached to the USB devices,
thus useful for any onboard / embedded USB devices that wants to get
some info from the ACPI).
Rajat Jain (3):
Bluetooth: Reset Bluetooth chip after multiple command timeouts
Bluetooth: btusb: Collect the common Intel assignments together
Bluetooth: btusb: Use the hw_reset method to allow resetting the BT
chip
drivers/bluetooth/btusb.c | 63 +++++++++---
drivers/usb/core/usb-acpi.c | 163 +++++++++++++++++++------------
include/net/bluetooth/hci.h | 8 ++
include/net/bluetooth/hci_core.h | 2 +
net/bluetooth/hci_core.c | 15 ++-
5 files changed, 171 insertions(+), 80 deletions(-)
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply
* Re: [PATCH net-next 6/8] net: eth: altera: tse: add support for ptp and timestamping
From: Dalon Westergreen @ 2018-11-16 14:48 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, dinguyen, thor.thayer
In-Reply-To: <20181116021418.dhbw5cu4y56cfjls@localhost>
On Thu, 2018-11-15 at 18:14 -0800, Richard Cochran wrote:
> On Thu, Nov 15, 2018 at 06:55:29AM -0800, Dalon Westergreen wrote:
> > I would prefer to keep altera just to be consistent with the altera_tse
> > stuff,
> > and i intend to reusethis code for a 10GbE driver, so perhaps altera_tod to
> > reference the fpga ip name?
>
> So the IP core is called "tod"? Really?
For naming, how about intel_fpga_tod ?
--dalon
>
> Thanks,
> Richard
^ permalink raw reply
* RE: [PATCH net-next 2/2] net/sched: act_police: don't use spinlock in the data path
From: David Laight @ 2018-11-16 14:41 UTC (permalink / raw)
To: 'Eric Dumazet', Davide Caratti, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, David S. Miller
Cc: netdev@vger.kernel.org
In-Reply-To: <7ba53062-de84-a8f1-14dc-3c49a2480925@gmail.com>
From: Eric Dumazet
> Sent: 16 November 2018 14:35
...
> I suggest to use a single cache line with a dedicated spinlock and these three s64
>
> spinlock_t tcfp_lock ____cacheline_aligned_in_smp;
> s64 ...
> s64 ...
> s64 ...
Doesn't this do something really stupid when cache lines are big.
If the spinlock is 8 bytes you never want more than 32 byte alignment.
If cache lines are 256 bytes you don't even need that.
Also ISTR that the kmalloc() only guarantees 8 byte alignment on x86_64.
So aligning structure members to larger offsets is rather pointless.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: KASAN poisoning for skb linear data
From: Dmitry Vyukov @ 2018-11-17 0:52 UTC (permalink / raw)
To: David Miller, Willem de Bruijn, Eric Dumazet, netdev, LKML,
kasan-dev, Cong Wang, andreyknvl
In-Reply-To: <CACT4Y+aLOqSqdfRYEyQB33MSXXb-3=APs5B1ZHsXJ2XYXF_Ppw@mail.gmail.com>
On Thu, Mar 8, 2018 at 1:20 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Jan 15, 2018 at 3:15 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> Hi,
>>
>> As far as I understand pskb_may_pull() plays important role in packet
>> parsing for all protocols. And we did custom fragmentation of packets
>> emitted via tun (IFF_NAPI_FRAGS). However, it seems that it does not
>> give any results (bugs found), and I think the reason for this is that
>> linear data is rounded up and is usually quite large. So if a parsing
>> function does pskb_may_pull(1), or does not do it at all, it can
>> usually access more and it will go unnoticed. KASAN has an ability to
>> do custom poisoning: it can poison/unpoison any memory range, and then
>> detect any reads/writes to that range. What do you think about adding
>> custom KASAN poisoning to pskb_may_pull() and switching it to
>> non-eager mode (pull only what was requested) under KASAN? Do you
>> think it has potential for finding important bugs? What amount of work
>> is this?
>
> Filed https://bugzilla.kernel.org/show_bug.cgi?id=199055 for this so
> it's not get lost.
Bringing this up after we discussed this with Dave on plumbers.
There are 2 strategies for making KASAN aware of exact skb linear
buffer semantics.
1. Just using kmalloc/free each time with precise size.
2. Using KASAN annotations:
void kasan_poison_shadow(const void *address, size_t size, u8 value);
void kasan_unpoison_shadow(const void *address, size_t size);
https://github.com/torvalds/linux/blob/master/mm/kasan/kasan.c#L57
https://github.com/torvalds/linux/blob/master/include/linux/kasan.h#L38
If we use annotations we can keep more of the existing skb logic.
But AFAIU this way we won't be able to detect all accesses after a
potential reallocation.
There are also annotations for explicit checks:
https://github.com/torvalds/linux/blob/master/include/linux/kasan-checks.h#L6
But not sure we need them here (maybe more appropriate for places
where KASAN does not see memory accesses e.g. a driver handing off a
packet to DMA).
I don't think it makes sense to make any more complex than necessary
in the name of performance, at least initially. This will be enabled
only under #ifdef KASAN.
If somebody gives us any prototype, we can assess (1) if it works and
(2) if it catches any new bugs.
Thanks
^ permalink raw reply
* Re: [PATCH net-next 2/2] net/sched: act_police: don't use spinlock in the data path
From: Eric Dumazet @ 2018-11-16 14:39 UTC (permalink / raw)
To: Davide Caratti, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
David S. Miller
Cc: netdev
In-Reply-To: <7ba53062-de84-a8f1-14dc-3c49a2480925@gmail.com>
On 11/16/2018 06:34 AM, Eric Dumazet wrote:
>
>> + s64 tcfp_toks;
>> + s64 tcfp_ptoks;
>> + s64 tcfp_t_c;
>
> I suggest to use a single cache line with a dedicated spinlock and these three s64
>
> spinlock_t tcfp_lock ____cacheline_aligned_in_smp;
> s64 ...
> s64 ...
> s64 ...
>
>
>> struct tcf_police_params __rcu *params;
>
> Make sure to use a different cache line for *params
>
> struct tcf_police_params __rcu *params ____cacheline_aligned_in_smp;
Or move it before the cacheline used by the lock and three s64,
since 'common' should be read-mostly. No need for a separate cache line.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox