* Re: [bpf-next,v4] samples: bpf: add max_pckt_size option at xdp_adjust_tail
From: Yonghong Song @ 2019-09-16 16:07 UTC (permalink / raw)
To: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org
In-Reply-To: <20190915124733.31134-1-danieltimlee@gmail.com>
On 9/15/19 1:47 PM, Daniel T. Lee wrote:
> Currently, at xdp_adjust_tail_kern.c, MAX_PCKT_SIZE is limited
> to 600. To make this size flexible, a new map 'pcktsz' is added.
>
> By updating new packet size to this map from the userland,
> xdp_adjust_tail_kern.o will use this value as a new max_pckt_size.
>
> If no '-P <MAX_PCKT_SIZE>' option is used, the size of maximum packet
> will be 600 as a default.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
>
> ---
> Changes in v4:
> - make pckt_size no less than ICMP_TOOBIG_SIZE
> - Fix code style
> Changes in v2:
> - Change the helper to fetch map from 'bpf_map__next' to
> 'bpf_object__find_map_fd_by_name'.
>
> samples/bpf/xdp_adjust_tail_kern.c | 23 +++++++++++++++++++----
> samples/bpf/xdp_adjust_tail_user.c | 28 ++++++++++++++++++++++------
> 2 files changed, 41 insertions(+), 10 deletions(-)
LGTM except a minor comments below.
Acked-by: Yonghong Song <yhs@fb.com>
bpf-next is closed. Please resubmit the patch once it is opened
in around 2 weeks.
>
> diff --git a/samples/bpf/xdp_adjust_tail_kern.c b/samples/bpf/xdp_adjust_tail_kern.c
> index 411fdb21f8bc..8869bbb160d2 100644
> --- a/samples/bpf/xdp_adjust_tail_kern.c
> +++ b/samples/bpf/xdp_adjust_tail_kern.c
> @@ -25,6 +25,13 @@
> #define ICMP_TOOBIG_SIZE 98
> #define ICMP_TOOBIG_PAYLOAD_SIZE 92
>
> +struct bpf_map_def SEC("maps") pcktsz = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(__u32),
> + .value_size = sizeof(__u32),
> + .max_entries = 1,
> +};
> +
> struct bpf_map_def SEC("maps") icmpcnt = {
> .type = BPF_MAP_TYPE_ARRAY,
> .key_size = sizeof(__u32),
> @@ -64,7 +71,8 @@ static __always_inline void ipv4_csum(void *data_start, int data_size,
> *csum = csum_fold_helper(*csum);
> }
>
> -static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
> +static __always_inline int send_icmp4_too_big(struct xdp_md *xdp,
> + __u32 max_pckt_size)
> {
> int headroom = (int)sizeof(struct iphdr) + (int)sizeof(struct icmphdr);
>
> @@ -92,7 +100,7 @@ static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
> orig_iph = data + off;
> icmp_hdr->type = ICMP_DEST_UNREACH;
> icmp_hdr->code = ICMP_FRAG_NEEDED;
> - icmp_hdr->un.frag.mtu = htons(MAX_PCKT_SIZE-sizeof(struct ethhdr));
> + icmp_hdr->un.frag.mtu = htons(max_pckt_size - sizeof(struct ethhdr));
> icmp_hdr->checksum = 0;
> ipv4_csum(icmp_hdr, ICMP_TOOBIG_PAYLOAD_SIZE, &csum);
> icmp_hdr->checksum = csum;
> @@ -118,14 +126,21 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
> {
> void *data_end = (void *)(long)xdp->data_end;
> void *data = (void *)(long)xdp->data;
> + __u32 max_pckt_size = MAX_PCKT_SIZE;
> int pckt_size = data_end - data;
> + __u32 *pckt_sz;
> + __u32 key = 0;
> int offset;
>
> - if (pckt_size > MAX_PCKT_SIZE) {
> + pckt_sz = bpf_map_lookup_elem(&pcktsz, &key);
> + if (pckt_sz && *pckt_sz)
> + max_pckt_size = *pckt_sz;
> +
> + if (pckt_size > max(max_pckt_size, ICMP_TOOBIG_SIZE)) {
> offset = pckt_size - ICMP_TOOBIG_SIZE;
> if (bpf_xdp_adjust_tail(xdp, 0 - offset))
> return XDP_PASS;
> - return send_icmp4_too_big(xdp);
> + return send_icmp4_too_big(xdp, max_pckt_size);
> }
> return XDP_PASS;
> }
> diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
> index a3596b617c4c..99e965c68054 100644
> --- a/samples/bpf/xdp_adjust_tail_user.c
> +++ b/samples/bpf/xdp_adjust_tail_user.c
> @@ -23,6 +23,7 @@
> #include "libbpf.h"
>
> #define STATS_INTERVAL_S 2U
> +#define MAX_PCKT_SIZE 600
>
> static int ifindex = -1;
> static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> @@ -72,6 +73,7 @@ static void usage(const char *cmd)
> printf("Usage: %s [...]\n", cmd);
> printf(" -i <ifname|ifindex> Interface\n");
> printf(" -T <stop-after-X-seconds> Default: 0 (forever)\n");
> + printf(" -P <MAX_PCKT_SIZE> Default: %u\n", MAX_PCKT_SIZE);
> printf(" -S use skb-mode\n");
> printf(" -N enforce native mode\n");
> printf(" -F force loading prog\n");
> @@ -85,13 +87,14 @@ int main(int argc, char **argv)
> .prog_type = BPF_PROG_TYPE_XDP,
> };
> unsigned char opt_flags[256] = {};
> - const char *optstr = "i:T:SNFh";
> + const char *optstr = "i:T:P:SNFh";
> struct bpf_prog_info info = {};
> __u32 info_len = sizeof(info);
> + __u32 max_pckt_size = 0;
> + __u32 key = 0;
> unsigned int kill_after_s = 0;
> int i, prog_fd, map_fd, opt;
> struct bpf_object *obj;
> - struct bpf_map *map;
> char filename[256];
> int err;
>
> @@ -110,6 +113,9 @@ int main(int argc, char **argv)
> case 'T':
> kill_after_s = atoi(optarg);
> break;
> + case 'P':
> + max_pckt_size = atoi(optarg);
> + break;
> case 'S':
> xdp_flags |= XDP_FLAGS_SKB_MODE;
> break;
> @@ -150,12 +156,22 @@ int main(int argc, char **argv)
> if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
> return 1;
>
> - map = bpf_map__next(NULL, obj);
> - if (!map) {
> - printf("finding a map in obj file failed\n");
> + /* update pcktsz map */
> + if (max_pckt_size) {
> + map_fd = bpf_object__find_map_fd_by_name(obj, "pcktsz");
> + if (map_fd < 0) {
> + printf("finding a pcktsz map in obj file failed\n");
> + return 1;
> + }
> + bpf_map_update_elem(map_fd, &key, &max_pckt_size, BPF_ANY);
> + }
> +
> + /* fetch icmpcnt map */
> + map_fd = bpf_object__find_map_fd_by_name(obj, "icmpcnt");
> + if (map_fd < 0) {
> + printf("finding a icmpcnt map in obj file failed\n");
> return 1;
> }
> - map_fd = bpf_map__fd(map);
>
> if (!prog_fd) {
> printf("load_bpf_file: %s\n", strerror(errno));
Could you move the 'if (!prog_fd) ...' right after 'bpf_prog_load_xattr'
for readability reason?
Could you also change the condition 'if (!prog_fd)' to 'if (prog_fd <
0)'? You need to mention this fix in your commit message as well.
^ permalink raw reply
* Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?
From: George McCollister @ 2019-09-16 15:40 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Florian Fainelli, netdev, Andrew Lunn, Heiner Kallweit
In-Reply-To: <20190914084856.GD13294@shell.armlinux.org.uk>
On Sat, Sep 14, 2019 at 3:49 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> > +Russell, Andrew, Heiner,
> >
> > On 9/13/2019 9:44 AM, George McCollister wrote:
> > > Every example of phylink SFP support I've seen is using an Ethernet
> > > MAC with native SGMII.
> > > Can phylink facilitate support of Fiber and Copper SFP modules
> > > connected to an RGMII MAC if all of the following are true?
> >
> > I don't think that use case has been presented before, but phylink
> > sounds like the tool that should help solve it. From your description
> > below, it sounds like all the pieces are there to support it. Is the
> > Ethernet MAC driver upstream?
>
> It has been presented, and it's something I've been trying to support
> for the last couple of years - in fact, I have patches in my tree that
> support a very similar scenario on the Macchiatobin with the 88x3310
> PHYs.
>
> > > 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> > > Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> > > you want to see a block diagram it's the first one here:
> > > https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
>
> As mentioned above, this is no different from the Macchiatobin,
> where we have:
>
> .-------- RJ45
> MAC ---- 88x3310 PHY
> `-------- SFP+
>
> except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.
>
> Note that you're abusing the term "SGMII". SGMII is a Cisco
> modification of the IEEE 802.3 1000BASE-X protocol. Fiber SFPs
> exclusively use 1000BASE-X protocol. However, some copper SFPs
> (with a RJ45) do use SGMII.
>
> > > 2) The 1G Ethernet driver has been converted to use phylink.
>
> This is not necessary for this scenario. The PHY driver needs to
> be updated to know about SFP though.
Excellent, this is exactly the information I was looking for. I had
started converting the Ethernet driver to phylink but there was still
a lot of work to do and I fear there was a significant potential for
regressions. I'll abandon that and see if I can get it to work by
making similar changes to the 1G Marvell PHY driver.
I'm assuming I must set the sfp property of the PHY in DT instead of the MAC.
>
> See:
>
> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ece56785ee0e9df40dc823fdc39ee74b4a7cd1c4
>
> as an example of the 88x3310 supporting a SFP+ cage. This patch is
> also necessary:
>
> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ef2d699397ca28c7f89e01cc9e5037989096a990
Perfect.
>
> and if anything is going to stand in the way of progress on this, it
> is likely to be that patch. I'll be attempting to post these after
> the next merge window (i.o.w. probably posting them in three weeks
> time.)
Please CC me on these patches as I'll follow your lead for what I do
on the 1G Marvell PHY driver. If you don't remember, that's fine, I
understand.
>
> --
> 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,
George
^ permalink raw reply
* [PATCH iproute2] link_xfrm: don't forcce to set phydev
From: Nicolas Dichtel @ 2019-09-16 15:36 UTC (permalink / raw)
To: stephen; +Cc: netdev, dsahern, julien.floret, Nicolas Dichtel, Matt Ellison
Since linux commit 22d6552f827e ("xfrm interface: fix management of
phydev"), phydev is not mandatory anymore.
Note that it also could be useful before the above commit to not force the
user to put a phydev (the kernel was checking it anyway).
For example, it was useful to not set it in case of x-netns, because the
phydev is not available in the current netns:
Before the patch:
$ ip netns add foo
$ ip link add xfrm1 type xfrm dev eth1 if_id 1
$ ip link set xfrm1 netns foo
$ ip -n foo link set xfrm1 type xfrm dev eth1 if_id 2
Cannot find device "eth1"
$ ip -n foo link set xfrm1 type xfrm if_id 2
must specify physical device
CC: Matt Ellison <matt@arroyo.io>
Fixes: 286446c1e8c7 ("ip: support for xfrm interfaces")
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=22d6552f827e
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
ip/link_xfrm.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/ip/link_xfrm.c b/ip/link_xfrm.c
index 7a3285b42045..a28f308d5610 100644
--- a/ip/link_xfrm.c
+++ b/ip/link_xfrm.c
@@ -17,7 +17,7 @@ static void xfrm_print_help(struct link_util *lu, int argc, char **argv,
FILE *f)
{
fprintf(f,
- "Usage: ... %-4s dev PHYS_DEV [ if_id IF-ID ]\n"
+ "Usage: ... %-4s dev [ PHYS_DEV ] [ if_id IF-ID ]\n"
"\n"
"Where: IF-ID := { 0x0..0xffffffff }\n",
lu->id);
@@ -46,12 +46,8 @@ static int xfrm_parse_opt(struct link_util *lu, int argc, char **argv,
argc--; argv++;
}
- if (link) {
+ if (link)
addattr32(n, 1024, IFLA_XFRM_LINK, link);
- } else {
- fprintf(stderr, "must specify physical device\n");
- return -1;
- }
return 0;
}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH net] ibmvnic: Warn unknown speed message only when carrier is present
From: Thomas Falcon @ 2019-09-16 15:21 UTC (permalink / raw)
To: Murilo Fossa Vicentini, netdev; +Cc: muvic, abdhalee
In-Reply-To: <20190916145037.77376-1-muvic@linux.ibm.com>
On 9/16/19 9:50 AM, Murilo Fossa Vicentini wrote:
> With commit 0655f9943df2 ("net/ibmvnic: Update carrier state after link
> state change") we are now able to detect when the carrier is properly
> present in the device, so only report an unexpected unknown speed when it
> is properly detected. Unknown speed is expected to be seen by the device
> in case the backing device has no link detected.
>
> Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Tested-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Signed-off-by: Murilo Fossa Vicentini <muvic@linux.ibm.com>
> ---
Thanks, Murilo!
Reviewed-by: Thomas Falcon <tlfalcon@linux.ibm.com>
> drivers/net/ethernet/ibm/ibmvnic.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5cb55ea671e3..3a6725daf7dc 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -4312,13 +4312,14 @@ static int handle_query_phys_parms_rsp(union ibmvnic_crq *crq,
> {
> struct net_device *netdev = adapter->netdev;
> int rc;
> + __be32 rspeed = cpu_to_be32(crq->query_phys_parms_rsp.speed);
>
> rc = crq->query_phys_parms_rsp.rc.code;
> if (rc) {
> netdev_err(netdev, "Error %d in QUERY_PHYS_PARMS\n", rc);
> return rc;
> }
> - switch (cpu_to_be32(crq->query_phys_parms_rsp.speed)) {
> + switch (rspeed) {
> case IBMVNIC_10MBPS:
> adapter->speed = SPEED_10;
> break;
> @@ -4344,8 +4345,8 @@ static int handle_query_phys_parms_rsp(union ibmvnic_crq *crq,
> adapter->speed = SPEED_100000;
> break;
> default:
> - netdev_warn(netdev, "Unknown speed 0x%08x\n",
> - cpu_to_be32(crq->query_phys_parms_rsp.speed));
> + if (netif_carrier_ok(netdev))
> + netdev_warn(netdev, "Unknown speed 0x%08x\n", rspeed);
> adapter->speed = SPEED_UNKNOWN;
> }
> if (crq->query_phys_parms_rsp.flags1 & IBMVNIC_FULL_DUPLEX)
^ permalink raw reply
* Re: net: phy: micrel KSZ9031 ifdown ifup issue
From: Andrew Lunn @ 2019-09-16 15:13 UTC (permalink / raw)
To: Paul Thomas; +Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <CAD56B7dF9Dqf1wwu=w60z0q+hkE5-noZRS4uuUfF4PhyNSa4Kw@mail.gmail.com>
> When it is in the good state I see that reg 0x01 is 0x796d where bit
> 1.2 reports 'Link is up' and bit 1.5 reports 'Auto-negotiation process
> complete'. However, once I get to the bad state (it may take several
> tries of ifdown, ifup to get there) then reg 0x01 is 0x7649 reporting
> 'Link is down' and 'Auto-negotiation process not completed'. This can
> be fixed by resetting the phy './phytool write eth0/3/0 0x9140'
>
> So, I guess that means the driver is doing what it is supposed to?
> Could we add quirk or something to reset the phy again from the driver
> if auto-negotiation doesn't complete with x seconds?
Hi Paul
Adding a timeout would make sense. But please try to hide all this
inside the PHY driver. Since it is being polled, the read_status()
should be called once per second, so you should be able to handle all
this inside that driver callback.
Andrew
^ permalink raw reply
* [PATCH net-next v3 3/3] mlxsw: spectrum_buffers: Add the ability to query the CPU port's shared buffer
From: Ido Schimmel @ 2019-09-16 15:04 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190916150422.28947-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
While debugging packet loss towards the CPU, it is useful to be able to
query the CPU port's shared buffer quotas and occupancy.
Since the CPU port has no ingress buffers, all the shared buffers ingress
information will be cleared.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../mellanox/mlxsw/spectrum_buffers.c | 41 +++++++++++++++----
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
index f1dbde73fa78..b9eeae37a4dc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
@@ -250,6 +250,10 @@ static int mlxsw_sp_sb_pm_occ_clear(struct mlxsw_sp *mlxsw_sp, u8 local_port,
&mlxsw_sp->sb_vals->pool_dess[pool_index];
char sbpm_pl[MLXSW_REG_SBPM_LEN];
+ if (local_port == MLXSW_PORT_CPU_PORT &&
+ des->dir == MLXSW_REG_SBXX_DIR_INGRESS)
+ return 0;
+
mlxsw_reg_sbpm_pack(sbpm_pl, local_port, des->pool, des->dir,
true, 0, 0);
return mlxsw_reg_trans_query(mlxsw_sp->core, MLXSW_REG(sbpm), sbpm_pl,
@@ -273,6 +277,10 @@ static int mlxsw_sp_sb_pm_occ_query(struct mlxsw_sp *mlxsw_sp, u8 local_port,
char sbpm_pl[MLXSW_REG_SBPM_LEN];
struct mlxsw_sp_sb_pm *pm;
+ if (local_port == MLXSW_PORT_CPU_PORT &&
+ des->dir == MLXSW_REG_SBXX_DIR_INGRESS)
+ return 0;
+
pm = mlxsw_sp_sb_pm_get(mlxsw_sp, local_port, pool_index);
mlxsw_reg_sbpm_pack(sbpm_pl, local_port, des->pool, des->dir,
false, 0, 0);
@@ -1197,6 +1205,11 @@ static void mlxsw_sp_sb_sr_occ_query_cb(struct mlxsw_core *mlxsw_core,
local_port < mlxsw_core_max_ports(mlxsw_core); local_port++) {
if (!mlxsw_sp->ports[local_port])
continue;
+ if (local_port == MLXSW_PORT_CPU_PORT) {
+ /* Ingress quotas are not supported for the CPU port */
+ masked_count++;
+ continue;
+ }
for (i = 0; i < MLXSW_SP_SB_ING_TC_COUNT; i++) {
cm = mlxsw_sp_sb_cm_get(mlxsw_sp, local_port, i,
MLXSW_REG_SBXX_DIR_INGRESS);
@@ -1232,7 +1245,7 @@ int mlxsw_sp_sb_occ_snapshot(struct mlxsw_core *mlxsw_core,
char *sbsr_pl;
u8 masked_count;
u8 local_port_1;
- u8 local_port = 0;
+ u8 local_port;
int i;
int err;
int err2;
@@ -1241,8 +1254,8 @@ int mlxsw_sp_sb_occ_snapshot(struct mlxsw_core *mlxsw_core,
if (!sbsr_pl)
return -ENOMEM;
+ local_port = MLXSW_PORT_CPU_PORT;
next_batch:
- local_port++;
local_port_1 = local_port;
masked_count = 0;
mlxsw_reg_sbsr_pack(sbsr_pl, false);
@@ -1253,7 +1266,11 @@ int mlxsw_sp_sb_occ_snapshot(struct mlxsw_core *mlxsw_core,
for (; local_port < mlxsw_core_max_ports(mlxsw_core); local_port++) {
if (!mlxsw_sp->ports[local_port])
continue;
- mlxsw_reg_sbsr_ingress_port_mask_set(sbsr_pl, local_port, 1);
+ if (local_port != MLXSW_PORT_CPU_PORT) {
+ /* Ingress quotas are not supported for the CPU port */
+ mlxsw_reg_sbsr_ingress_port_mask_set(sbsr_pl,
+ local_port, 1);
+ }
mlxsw_reg_sbsr_egress_port_mask_set(sbsr_pl, local_port, 1);
for (i = 0; i < mlxsw_sp->sb_vals->pool_count; i++) {
err = mlxsw_sp_sb_pm_occ_query(mlxsw_sp, local_port, i,
@@ -1274,8 +1291,10 @@ int mlxsw_sp_sb_occ_snapshot(struct mlxsw_core *mlxsw_core,
cb_priv);
if (err)
goto out;
- if (local_port < mlxsw_core_max_ports(mlxsw_core))
+ if (local_port < mlxsw_core_max_ports(mlxsw_core)) {
+ local_port++;
goto next_batch;
+ }
out:
err2 = mlxsw_reg_trans_bulk_wait(&bulk_list);
@@ -1292,7 +1311,7 @@ int mlxsw_sp_sb_occ_max_clear(struct mlxsw_core *mlxsw_core,
LIST_HEAD(bulk_list);
char *sbsr_pl;
unsigned int masked_count;
- u8 local_port = 0;
+ u8 local_port;
int i;
int err;
int err2;
@@ -1301,8 +1320,8 @@ int mlxsw_sp_sb_occ_max_clear(struct mlxsw_core *mlxsw_core,
if (!sbsr_pl)
return -ENOMEM;
+ local_port = MLXSW_PORT_CPU_PORT;
next_batch:
- local_port++;
masked_count = 0;
mlxsw_reg_sbsr_pack(sbsr_pl, true);
for (i = 0; i < MLXSW_SP_SB_ING_TC_COUNT; i++)
@@ -1312,7 +1331,11 @@ int mlxsw_sp_sb_occ_max_clear(struct mlxsw_core *mlxsw_core,
for (; local_port < mlxsw_core_max_ports(mlxsw_core); local_port++) {
if (!mlxsw_sp->ports[local_port])
continue;
- mlxsw_reg_sbsr_ingress_port_mask_set(sbsr_pl, local_port, 1);
+ if (local_port != MLXSW_PORT_CPU_PORT) {
+ /* Ingress quotas are not supported for the CPU port */
+ mlxsw_reg_sbsr_ingress_port_mask_set(sbsr_pl,
+ local_port, 1);
+ }
mlxsw_reg_sbsr_egress_port_mask_set(sbsr_pl, local_port, 1);
for (i = 0; i < mlxsw_sp->sb_vals->pool_count; i++) {
err = mlxsw_sp_sb_pm_occ_clear(mlxsw_sp, local_port, i,
@@ -1329,8 +1352,10 @@ int mlxsw_sp_sb_occ_max_clear(struct mlxsw_core *mlxsw_core,
&bulk_list, NULL, 0);
if (err)
goto out;
- if (local_port < mlxsw_core_max_ports(mlxsw_core))
+ if (local_port < mlxsw_core_max_ports(mlxsw_core)) {
+ local_port++;
goto next_batch;
+ }
out:
err2 = mlxsw_reg_trans_bulk_wait(&bulk_list);
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v3 2/3] mlxsw: spectrum: Register CPU port with devlink
From: Ido Schimmel @ 2019-09-16 15:04 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190916150422.28947-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
Register CPU port with devlink.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 63 ++++++++++++++++---
drivers/net/ethernet/mellanox/mlxsw/core.h | 5 ++
.../net/ethernet/mellanox/mlxsw/spectrum.c | 46 ++++++++++++++
3 files changed, 105 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 3fa96076e8a5..14dcc786926d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1864,11 +1864,12 @@ u64 mlxsw_core_res_get(struct mlxsw_core *mlxsw_core,
}
EXPORT_SYMBOL(mlxsw_core_res_get);
-int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
- u32 port_number, bool split,
- u32 split_port_subnumber,
- const unsigned char *switch_id,
- unsigned char switch_id_len)
+static int __mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
+ enum devlink_port_flavour flavour,
+ u32 port_number, bool split,
+ u32 split_port_subnumber,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len)
{
struct devlink *devlink = priv_to_devlink(mlxsw_core);
struct mlxsw_core_port *mlxsw_core_port =
@@ -1877,17 +1878,16 @@ int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
int err;
mlxsw_core_port->local_port = local_port;
- devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
- port_number, split, split_port_subnumber,
+ devlink_port_attrs_set(devlink_port, flavour, port_number,
+ split, split_port_subnumber,
switch_id, switch_id_len);
err = devlink_port_register(devlink, devlink_port, local_port);
if (err)
memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
return err;
}
-EXPORT_SYMBOL(mlxsw_core_port_init);
-void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port)
+static void __mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port)
{
struct mlxsw_core_port *mlxsw_core_port =
&mlxsw_core->ports[local_port];
@@ -1896,8 +1896,53 @@ void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port)
devlink_port_unregister(devlink_port);
memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
}
+
+int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
+ u32 port_number, bool split,
+ u32 split_port_subnumber,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len)
+{
+ return __mlxsw_core_port_init(mlxsw_core, local_port,
+ DEVLINK_PORT_FLAVOUR_PHYSICAL,
+ port_number, split, split_port_subnumber,
+ switch_id, switch_id_len);
+}
+EXPORT_SYMBOL(mlxsw_core_port_init);
+
+void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port)
+{
+ __mlxsw_core_port_fini(mlxsw_core, local_port);
+}
EXPORT_SYMBOL(mlxsw_core_port_fini);
+int mlxsw_core_cpu_port_init(struct mlxsw_core *mlxsw_core,
+ void *port_driver_priv,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len)
+{
+ struct mlxsw_core_port *mlxsw_core_port =
+ &mlxsw_core->ports[MLXSW_PORT_CPU_PORT];
+ int err;
+
+ err = __mlxsw_core_port_init(mlxsw_core, MLXSW_PORT_CPU_PORT,
+ DEVLINK_PORT_FLAVOUR_CPU,
+ 0, false, 0,
+ switch_id, switch_id_len);
+ if (err)
+ return err;
+
+ mlxsw_core_port->port_driver_priv = port_driver_priv;
+ return 0;
+}
+EXPORT_SYMBOL(mlxsw_core_cpu_port_init);
+
+void mlxsw_core_cpu_port_fini(struct mlxsw_core *mlxsw_core)
+{
+ __mlxsw_core_port_fini(mlxsw_core, MLXSW_PORT_CPU_PORT);
+}
+EXPORT_SYMBOL(mlxsw_core_cpu_port_fini);
+
void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
void *port_driver_priv, struct net_device *dev)
{
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index b65a17d49e43..5d7d2ab6d155 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -177,6 +177,11 @@ int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
const unsigned char *switch_id,
unsigned char switch_id_len);
void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port);
+int mlxsw_core_cpu_port_init(struct mlxsw_core *mlxsw_core,
+ void *port_driver_priv,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len);
+void mlxsw_core_cpu_port_fini(struct mlxsw_core *mlxsw_core);
void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
void *port_driver_priv, struct net_device *dev);
void mlxsw_core_port_ib_set(struct mlxsw_core *mlxsw_core, u8 local_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 91e4792bb7e7..dd234cf7b39d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3872,6 +3872,45 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
mlxsw_core_port_fini(mlxsw_sp->core, local_port);
}
+static int mlxsw_sp_cpu_port_create(struct mlxsw_sp *mlxsw_sp)
+{
+ struct mlxsw_sp_port *mlxsw_sp_port;
+ int err;
+
+ mlxsw_sp_port = kzalloc(sizeof(*mlxsw_sp_port), GFP_KERNEL);
+ if (!mlxsw_sp_port)
+ return -ENOMEM;
+
+ mlxsw_sp_port->mlxsw_sp = mlxsw_sp;
+ mlxsw_sp_port->local_port = MLXSW_PORT_CPU_PORT;
+
+ err = mlxsw_core_cpu_port_init(mlxsw_sp->core,
+ mlxsw_sp_port,
+ mlxsw_sp->base_mac,
+ sizeof(mlxsw_sp->base_mac));
+ if (err) {
+ dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize core CPU port\n");
+ goto err_core_cpu_port_init;
+ }
+
+ mlxsw_sp->ports[MLXSW_PORT_CPU_PORT] = mlxsw_sp_port;
+ return 0;
+
+err_core_cpu_port_init:
+ kfree(mlxsw_sp_port);
+ return err;
+}
+
+static void mlxsw_sp_cpu_port_remove(struct mlxsw_sp *mlxsw_sp)
+{
+ struct mlxsw_sp_port *mlxsw_sp_port =
+ mlxsw_sp->ports[MLXSW_PORT_CPU_PORT];
+
+ mlxsw_core_cpu_port_fini(mlxsw_sp->core);
+ mlxsw_sp->ports[MLXSW_PORT_CPU_PORT] = NULL;
+ kfree(mlxsw_sp_port);
+}
+
static bool mlxsw_sp_port_created(struct mlxsw_sp *mlxsw_sp, u8 local_port)
{
return mlxsw_sp->ports[local_port] != NULL;
@@ -3884,6 +3923,7 @@ static void mlxsw_sp_ports_remove(struct mlxsw_sp *mlxsw_sp)
for (i = 1; i < mlxsw_core_max_ports(mlxsw_sp->core); i++)
if (mlxsw_sp_port_created(mlxsw_sp, i))
mlxsw_sp_port_remove(mlxsw_sp, i);
+ mlxsw_sp_cpu_port_remove(mlxsw_sp);
kfree(mlxsw_sp->port_to_module);
kfree(mlxsw_sp->ports);
}
@@ -3908,6 +3948,10 @@ static int mlxsw_sp_ports_create(struct mlxsw_sp *mlxsw_sp)
goto err_port_to_module_alloc;
}
+ err = mlxsw_sp_cpu_port_create(mlxsw_sp);
+ if (err)
+ goto err_cpu_port_create;
+
for (i = 1; i < max_ports; i++) {
/* Mark as invalid */
mlxsw_sp->port_to_module[i] = -1;
@@ -3931,6 +3975,8 @@ static int mlxsw_sp_ports_create(struct mlxsw_sp *mlxsw_sp)
for (i--; i >= 1; i--)
if (mlxsw_sp_port_created(mlxsw_sp, i))
mlxsw_sp_port_remove(mlxsw_sp, i);
+ mlxsw_sp_cpu_port_remove(mlxsw_sp);
+err_cpu_port_create:
kfree(mlxsw_sp->port_to_module);
err_port_to_module_alloc:
kfree(mlxsw_sp->ports);
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v3 1/3] mlxsw: spectrum_buffers: Prevent changing CPU port's configuration
From: Ido Schimmel @ 2019-09-16 15:04 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190916150422.28947-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
Next patch is going to register the CPU port with devlink, but only so
that the CPU port's shared buffer configuration and occupancy could be
queried.
Prevent changing CPU port's shared buffer threshold and binding
configuration.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
index 888ba4300bcc..f1dbde73fa78 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
@@ -1085,6 +1085,11 @@ int mlxsw_sp_sb_port_pool_set(struct mlxsw_core_port *mlxsw_core_port,
u32 max_buff;
int err;
+ if (local_port == MLXSW_PORT_CPU_PORT) {
+ NL_SET_ERR_MSG_MOD(extack, "Changing CPU port's threshold is forbidden");
+ return -EINVAL;
+ }
+
err = mlxsw_sp_sb_threshold_in(mlxsw_sp, pool_index,
threshold, &max_buff, extack);
if (err)
@@ -1130,6 +1135,11 @@ int mlxsw_sp_sb_tc_pool_bind_set(struct mlxsw_core_port *mlxsw_core_port,
u32 max_buff;
int err;
+ if (local_port == MLXSW_PORT_CPU_PORT) {
+ NL_SET_ERR_MSG_MOD(extack, "Changing CPU port's binding is forbidden");
+ return -EINVAL;
+ }
+
if (dir != mlxsw_sp->sb_vals->pool_dess[pool_index].dir) {
NL_SET_ERR_MSG_MOD(extack, "Binding egress TC to ingress pool and vice versa is forbidden");
return -EINVAL;
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v3 0/3] mlxsw: spectrum_buffers: Add the ability to query the CPU port's shared buffer
From: Ido Schimmel @ 2019-09-16 15:04 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, shalomt, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Shalom says:
While debugging packet loss towards the CPU, it is useful to be able to
query the CPU port's shared buffer quotas and occupancy.
Patch #1 prevents changing the CPU port's threshold and binding.
Patch #2 registers the CPU port with devlink.
Patch #3 adds the ability to query the CPU port's shared buffer quotas and
occupancy.
v3:
Patch #2:
* Remove unnecessary wrapping
v2:
Patch #1:
* s/0/MLXSW_PORT_CPU_PORT/
* Assign "mlxsw_sp->ports[MLXSW_PORT_CPU_PORT]" at the end of
mlxsw_sp_cpu_port_create() to avoid NULL assignment on error path
* Add common functions for mlxsw_core_port_init/fini()
Patch #2:
* Move "changing CPU port's threshold and binding" check to a separate
patch
Shalom Toledo (3):
mlxsw: spectrum_buffers: Prevent changing CPU port's configuration
mlxsw: spectrum: Register CPU port with devlink
mlxsw: spectrum_buffers: Add the ability to query the CPU port's
shared buffer
drivers/net/ethernet/mellanox/mlxsw/core.c | 63 ++++++++++++++++---
drivers/net/ethernet/mellanox/mlxsw/core.h | 5 ++
.../net/ethernet/mellanox/mlxsw/spectrum.c | 46 ++++++++++++++
.../mellanox/mlxsw/spectrum_buffers.c | 51 ++++++++++++---
4 files changed, 148 insertions(+), 17 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [PATCH net v3 03/11] bonding: fix unexpected IFF_BONDING bit unset
From: Jay Vosburgh @ 2019-09-16 15:03 UTC (permalink / raw)
To: Jiri Pirko
Cc: Taehee Yoo, davem, netdev, vfalico, andy, sd, roopa, saeedm,
manishc, rahulv, kys, haiyangz, stephen, sashal, hare, varun,
ubraun, kgraul
In-Reply-To: <20190916144930.GO2286@nanopsycho.orion>
Jiri Pirko <jiri@resnulli.us> wrote:
>Mon, Sep 16, 2019 at 03:47:54PM CEST, ap420073@gmail.com wrote:
>>The IFF_BONDING means bonding master or bonding slave device.
>>->ndo_add_slave() sets IFF_BONDING flag and ->ndo_del_slave() unsets
>>IFF_BONDING flag.
>>
>>bond0<--bond1
>>
>>Both bond0 and bond1 are bonding device and these should keep having
>>IFF_BONDING flag until they are removed.
>>But bond1 would lose IFF_BONDING at ->ndo_del_slave() because that routine
>>do not check whether the slave device is the bonding type or not.
>>This patch adds the interface type check routine before removing
>>IFF_BONDING flag.
>>
>>Test commands:
>> ip link add bond0 type bond
>> ip link add bond1 type bond
>> ip link set bond1 master bond0
>> ip link set bond1 nomaster
>> ip link del bond1 type bond
>> ip link add bond1 type bond
>
>Interesting. I wonder why bond-in-bond is not forbidden...
I think mostly because nesting wasn't originally forbidden, and
there are apparently users of it out in the wild, judging from the
number of times I see configurations or queries about an active-backup
bond with two 802.3ad bonding slaves. That particular configuration
doesn't have any advantage (802.3ad will internally manage that
situation), but I don't see that we can now forbid nesting bonds without
potentially breaking existing user space configurations.
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* [PATCH] selftests/net: replace AF_MAX with INT_MAX in socket.c
From: Marcelo Henrique Cerri @ 2019-09-16 15:03 UTC (permalink / raw)
To: David S. Miller, Shuah Khan; +Cc: netdev, linux-kselftest, linux-kernel
Use INT_MAX instead of AF_MAX, since libc might have a smaller value
of AF_MAX than the kernel, what causes the test to fail.
Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
---
tools/testing/selftests/net/socket.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/socket.c b/tools/testing/selftests/net/socket.c
index afca1ead677f..10e75ba90124 100644
--- a/tools/testing/selftests/net/socket.c
+++ b/tools/testing/selftests/net/socket.c
@@ -6,6 +6,7 @@
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
+#include <limits.h>
struct socket_testcase {
int domain;
@@ -24,7 +25,10 @@ struct socket_testcase {
};
static struct socket_testcase tests[] = {
- { AF_MAX, 0, 0, -EAFNOSUPPORT, 0 },
+ /* libc might have a smaller value of AF_MAX than the kernel
+ * actually supports, so use INT_MAX instead.
+ */
+ { INT_MAX, 0, 0, -EAFNOSUPPORT, 0 },
{ AF_INET, SOCK_STREAM, IPPROTO_TCP, 0, 1 },
{ AF_INET, SOCK_DGRAM, IPPROTO_TCP, -EPROTONOSUPPORT, 1 },
{ AF_INET, SOCK_DGRAM, IPPROTO_UDP, 0, 1 },
--
2.20.1
^ permalink raw reply related
* Re: [bisected] UDP / xfrm: NAT-T packets with bad UDP checksum get dropped
From: Thomas Jarosch @ 2019-09-16 14:45 UTC (permalink / raw)
To: netdev; +Cc: Tom Herbert, Steffen Klassert, Sean Tranchetti, Eric Dumazet
In-Reply-To: <20190912151353.2w7jakdrqljkfbsq@intra2net.com>
> After a few hours of bisecting with a test VM,
> this commit was identified to cause the packet drop:
>
> *******************
> commit 0a80966b1043c3e2dc684140f155a3fded308660
> Author: Tom Herbert <therbert@google.com>
> Date: Wed May 7 16:52:39 2014 -0700
>
> net: Verify UDP checksum before handoff to encap
>
> Moving validation of UDP checksum to be done in UDP not encap layer.
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index f2d05d7be743..54ea0a3a48f1 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1495,6 +1495,10 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> if (skb->len > sizeof(struct udphdr) && encap_rcv != NULL) {
> int ret;
>
> + /* Verify checksum before giving to encap */
> + if (udp_lib_checksum_complete(skb))
> + goto csum_error;
> +
> ret = encap_rcv(sk, skb);
> if (ret <= 0) {
> UDP_INC_STATS_BH(sock_net(sk),
> ..
> *******************
>
> This commit is part of kernel 3.16. Reverting the commit
> brings back the VPN connection using kernel 4.19.67.
..brings back the VPN connection when there's no iptables firewall involved ^^
As soon as netfilter is active, the packets get eaten again.
I've bisected this down to another commit, basically it's "broken" in 4.19.1
and works with with 4.19 vanilla. The commit in question:
*************************************
# git bisect good
4fb0dc97de1cd79399ab0c556f096d8db2bac278 is the first bad commit
commit 4fb0dc97de1cd79399ab0c556f096d8db2bac278
Author: Sean Tranchetti <stranche@codeaurora.org>
Date: Tue Oct 23 16:04:31 2018 -0600
net: udp: fix handling of CHECKSUM_COMPLETE packets
[ Upstream commit db4f1be3ca9b0ef7330763d07bf4ace83ad6f913 ]
Current handling of CHECKSUM_COMPLETE packets by the UDP stack is
incorrect for any packet that has an incorrect checksum value.
udp4/6_csum_init() will both make a call to
__skb_checksum_validate_complete() to initialize/validate the csum
field when receiving a CHECKSUM_COMPLETE packet. When this packet
fails validation, skb->csum will be overwritten with the pseudoheader
checksum so the packet can be fully validated by software, but the
skb->ip_summed value will be left as CHECKSUM_COMPLETE so that way
the stack can later warn the user about their hardware spewing bad
checksums. Unfortunately, leaving the SKB in this state can cause
problems later on in the checksum calculation.
Since the the packet is still marked as CHECKSUM_COMPLETE,
udp_csum_pull_header() will SUBTRACT the checksum of the UDP header
from skb->csum instead of adding it, leaving us with a garbage value
in that field. Once we try to copy the packet to userspace in the
udp4/6_recvmsg(), we'll make a call to skb_copy_and_csum_datagram_msg()
to checksum the packet data and add it in the garbage skb->csum value
to perform our final validation check.
Since the value we're validating is not the proper checksum, it's possible
that the folded value could come out to 0, causing us not to drop the
packet. Instead, we believe that the packet was checksummed incorrectly
by hardware since skb->ip_summed is still CHECKSUM_COMPLETE, and we attempt
to warn the user with netdev_rx_csum_fault(skb->dev);
Unfortunately, since this is the UDP path, skb->dev has been overwritten
by skb->dev_scratch and is no longer a valid pointer, so we end up
reading invalid memory.
This patch addresses this problem in two ways:
1) Do not use the dev pointer when calling netdev_rx_csum_fault()
from skb_copy_and_csum_datagram_msg(). Since this gets called
from the UDP path where skb->dev has been overwritten, we have
no way of knowing if the pointer is still valid. Also for the
sake of consistency with the other uses of
netdev_rx_csum_fault(), don't attempt to call it if the
packet was checksummed by software.
2) Add better CHECKSUM_COMPLETE handling to udp4/6_csum_init().
If we receive a packet that's CHECKSUM_COMPLETE that fails
verification (i.e. skb->csum_valid == 0), check who performed
the calculation. It's possible that the checksum was done in
software by the network stack earlier (such as Netfilter's
CONNTRACK module), and if that says the checksum is bad,
we can drop the packet immediately instead of waiting until
we try and copy it to userspace. Otherwise, we need to
mark the SKB as CHECKSUM_NONE, since the skb->csum field
no longer contains the full packet checksum after the
call to __skb_checksum_validate_complete().
Fixes: e6afc8ace6dd ("udp: remove headers from UDP packets before queueing")
Fixes: c84d949057ca ("udp: copy skb->truesize in the first cache line")
Cc: Sam Kumar <samanthakumar@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
*************************************
Reverting this commit and the previously mentioned one
0a80966b1043c3e2dc684140f155a3fded308660
("net: Verify UDP checksum before handoff to encap")
brings back the VPN tunnel using 4.19.57 including the full iptables firewall.
(I had the firewall disabled during my earlier tests)
Given the patch description above, this commit is not something I want to revert.
I tried playing around with the CHECKSUM netfilter target, but it just seems
to fill in missing checksums, not overwrite invalid ones.
My next step is to figure out what brand and model exactly the
"unknown home router" is and if there's a firmware update available.
If it can be fixed by replacing / upgrading the home router, I will go this
route and cross my fingers this will by a one time VPN tunnel incident
after the major kernel 3.14 -> 4.19 upgrade.
Cheers,
Thomas
^ permalink raw reply
* Re: [PATCH net-next] net: phylink: clarify where phylink should be used
From: David Miller @ 2019-09-16 14:54 UTC (permalink / raw)
To: rmk+kernel; +Cc: andrew, f.fainelli, hkallweit1, corbet, netdev, linux-doc
In-Reply-To: <E1i94b6-0008TL-IR@rmk-PC.armlinux.org.uk>
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Sat, 14 Sep 2019 10:44:04 +0100
> Update the phylink documentation to make it clear that phylink is
> designed to be used on the MAC facing side of the link, rather than
> between a SFP and PHY.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied.
^ permalink raw reply
* Re: [patch net-next 02/15] net: fib_notifier: make FIB notifier per-netns
From: David Ahern @ 2019-09-16 14:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, jakub.kicinski, tariqt, saeedm, kuznet,
yoshfuji, shuah, mlxsw
In-Reply-To: <20190916053801.GG2286@nanopsycho.orion>
On 9/15/19 11:38 PM, Jiri Pirko wrote:
>> This is still more complicated than it needs to be. Why lump all
>> fib_notifier_ops into 1 dump when they are separate databases with
>> separate seq numbers? Just dump them 1 at a time and retry that 1
>> database as needed.
>
> Well I think that what you describe is out of scope of this patch. It is
> another optimization of fib_notifier. The aim of this patchset is not
> optimization of fib_notifier, but devlink netns change. This patchset is
> just a dependency.
>
> Can't we do optimization in another patchset? I already struggled to
> keep this one within 15-patch limit.
>
sure, but it seems to me it is less work to churn this code all at once
and you are already doing the testing. The fib notifier changes can
always be done as a prep set.
^ permalink raw reply
* [PATCH net] ibmvnic: Warn unknown speed message only when carrier is present
From: Murilo Fossa Vicentini @ 2019-09-16 14:50 UTC (permalink / raw)
To: netdev; +Cc: tlfalcon, muvic, abdhalee
With commit 0655f9943df2 ("net/ibmvnic: Update carrier state after link
state change") we are now able to detect when the carrier is properly
present in the device, so only report an unexpected unknown speed when it
is properly detected. Unknown speed is expected to be seen by the device
in case the backing device has no link detected.
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Tested-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Signed-off-by: Murilo Fossa Vicentini <muvic@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5cb55ea671e3..3a6725daf7dc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -4312,13 +4312,14 @@ static int handle_query_phys_parms_rsp(union ibmvnic_crq *crq,
{
struct net_device *netdev = adapter->netdev;
int rc;
+ __be32 rspeed = cpu_to_be32(crq->query_phys_parms_rsp.speed);
rc = crq->query_phys_parms_rsp.rc.code;
if (rc) {
netdev_err(netdev, "Error %d in QUERY_PHYS_PARMS\n", rc);
return rc;
}
- switch (cpu_to_be32(crq->query_phys_parms_rsp.speed)) {
+ switch (rspeed) {
case IBMVNIC_10MBPS:
adapter->speed = SPEED_10;
break;
@@ -4344,8 +4345,8 @@ static int handle_query_phys_parms_rsp(union ibmvnic_crq *crq,
adapter->speed = SPEED_100000;
break;
default:
- netdev_warn(netdev, "Unknown speed 0x%08x\n",
- cpu_to_be32(crq->query_phys_parms_rsp.speed));
+ if (netif_carrier_ok(netdev))
+ netdev_warn(netdev, "Unknown speed 0x%08x\n", rspeed);
adapter->speed = SPEED_UNKNOWN;
}
if (crq->query_phys_parms_rsp.flags1 & IBMVNIC_FULL_DUPLEX)
--
2.20.1 (Apple Git-117)
^ permalink raw reply related
* Re: [PATCH net v3 03/11] bonding: fix unexpected IFF_BONDING bit unset
From: Jiri Pirko @ 2019-09-16 14:49 UTC (permalink / raw)
To: Taehee Yoo
Cc: davem, netdev, j.vosburgh, vfalico, andy, sd, roopa, saeedm,
manishc, rahulv, kys, haiyangz, stephen, sashal, hare, varun,
ubraun, kgraul, jay.vosburgh
In-Reply-To: <20190916134802.8252-4-ap420073@gmail.com>
Mon, Sep 16, 2019 at 03:47:54PM CEST, ap420073@gmail.com wrote:
>The IFF_BONDING means bonding master or bonding slave device.
>->ndo_add_slave() sets IFF_BONDING flag and ->ndo_del_slave() unsets
>IFF_BONDING flag.
>
>bond0<--bond1
>
>Both bond0 and bond1 are bonding device and these should keep having
>IFF_BONDING flag until they are removed.
>But bond1 would lose IFF_BONDING at ->ndo_del_slave() because that routine
>do not check whether the slave device is the bonding type or not.
>This patch adds the interface type check routine before removing
>IFF_BONDING flag.
>
>Test commands:
> ip link add bond0 type bond
> ip link add bond1 type bond
> ip link set bond1 master bond0
> ip link set bond1 nomaster
> ip link del bond1 type bond
> ip link add bond1 type bond
Interesting. I wonder why bond-in-bond is not forbidden...
^ permalink raw reply
* Re: [patch iproute2-next v4 0/2] devlink: couple forgotten flash patches
From: David Ahern @ 2019-09-16 14:49 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, stephen, jakub.kicinski, saeedm, mlxsw, f.fainelli
In-Reply-To: <20190916100924.GM2286@nanopsycho.orion>
On 9/16/19 4:09 AM, Jiri Pirko wrote:
> The fact that the file is expected to be in /lib/firmware is in the
> devlink flash description right above:
>
>
> devlink dev flash - write device's non-volatile memory.
> DEV - specifies the devlink device to write to.
>
> file PATH - Path to the file which will be written into device's flash. The path needs to be relative to one of the directories
> searched by the kernel firmware loaded, such as /lib/firmware.
fine. applied both to iproute2-next.
^ permalink raw reply
* Re: [PATCH v5 2/2] tcp: Add snd_wnd to TCP_INFO
From: David Miller @ 2019-09-16 14:39 UTC (permalink / raw)
To: tph
Cc: netdev, jonathan.lemon, dsj, edumazet, ncardwell, dave.taht,
ycheng, soheil
In-Reply-To: <20190913232332.44036-2-tph@fb.com>
From: Thomas Higdon <tph@fb.com>
Date: Fri, 13 Sep 2019 23:23:35 +0000
> Neal Cardwell mentioned that snd_wnd would be useful for diagnosing TCP
> performance problems --
>> (1) Usually when we're diagnosing TCP performance problems, we do so
>> from the sender, since the sender makes most of the
>> performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc).
>> From the sender-side the thing that would be most useful is to see
>> tp->snd_wnd, the receive window that the receiver has advertised to
>> the sender.
>
> This serves the purpose of adding an additional __u32 to avoid the
> would-be hole caused by the addition of the tcpi_rcvi_ooopack field.
>
> Signed-off-by: Thomas Higdon <tph@fb.com>
Applied.
^ permalink raw reply
* Re: [PATCH v5 1/2] tcp: Add TCP_INFO counter for packets received out-of-order
From: David Miller @ 2019-09-16 14:39 UTC (permalink / raw)
To: tph
Cc: netdev, jonathan.lemon, dsj, edumazet, ncardwell, dave.taht,
ycheng, soheil
In-Reply-To: <20190913232332.44036-1-tph@fb.com>
From: Thomas Higdon <tph@fb.com>
Date: Fri, 13 Sep 2019 23:23:34 +0000
> For receive-heavy cases on the server-side, we want to track the
> connection quality for individual client IPs. This counter, similar to
> the existing system-wide TCPOFOQueue counter in /proc/net/netstat,
> tracks out-of-order packet reception. By providing this counter in
> TCP_INFO, it will allow understanding to what degree receive-heavy
> sockets are experiencing out-of-order delivery and packet drops
> indicating congestion.
>
> Please note that this is similar to the counter in NetBSD TCP_INFO, and
> has the same name.
>
> Also note that we avoid increasing the size of the tcp_sock struct by
> taking advantage of a hole.
>
> Signed-off-by: Thomas Higdon <tph@fb.com>
Applied.
^ permalink raw reply
* Re: [PATCH net v3 03/11] bonding: fix unexpected IFF_BONDING bit unset
From: Jay Vosburgh @ 2019-09-16 14:16 UTC (permalink / raw)
To: Taehee Yoo
Cc: davem, netdev, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul
In-Reply-To: <20190916134802.8252-4-ap420073@gmail.com>
Taehee Yoo <ap420073@gmail.com> wrote:
>The IFF_BONDING means bonding master or bonding slave device.
>->ndo_add_slave() sets IFF_BONDING flag and ->ndo_del_slave() unsets
>IFF_BONDING flag.
>
>bond0<--bond1
>
>Both bond0 and bond1 are bonding device and these should keep having
>IFF_BONDING flag until they are removed.
>But bond1 would lose IFF_BONDING at ->ndo_del_slave() because that routine
>do not check whether the slave device is the bonding type or not.
>This patch adds the interface type check routine before removing
>IFF_BONDING flag.
>
>Test commands:
> ip link add bond0 type bond
> ip link add bond1 type bond
> ip link set bond1 master bond0
> ip link set bond1 nomaster
> ip link del bond1 type bond
> ip link add bond1 type bond
>
>Splat looks like:
>[ 58.210981] proc_dir_entry 'bonding/bond1' already registered
>[ 58.463875] WARNING: CPU: 0 PID: 955 at fs/proc/generic.c:361 proc_register+0x2a9/0x3e0
>[ 58.466423] Modules linked in: bonding veth openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nfs
>[ 58.483855] CPU: 0 PID: 955 Comm: ip Not tainted 5.3.0-rc8+ #179
>[ 58.484657] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>[ 58.485779] RIP: 0010:proc_register+0x2a9/0x3e0
>[ 58.486377] Code: 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 39 01 00 00 48 8b 04 24 48 89 ea 48 c7 c7 60 0f 14 bd 480
>[ 58.489003] RSP: 0018:ffff8880cc007078 EFLAGS: 00010282
>[ 58.553743] RAX: dffffc0000000008 RBX: ffff8880ce23c0d0 RCX: ffffffffbbd021e2
>[ 58.584076] RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffff8880da5f6b8c
>[ 58.584901] RBP: ffff8880ce23c353 R08: ffffed101b4bff91 R09: ffffed101b4bff91
>[ 58.585724] R10: 0000000000000001 R11: ffffed101b4bff90 R12: ffff8880ce23c268
>[ 58.586508] R13: ffff8880ce23c352 R14: dffffc0000000000 R15: ffffed1019c4786a
>[ 58.587296] FS: 00007f52d53b60c0(0000) GS:ffff8880da400000(0000) knlGS:0000000000000000
>[ 58.588247] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[ 58.653694] CR2: 00007f31a9df9320 CR3: 00000000cd4ea006 CR4: 00000000000606f0
>[ 58.654591] Call Trace:
>[ 58.654895] proc_create_seq_private+0xb3/0xf0
>[ 58.655400] bond_create_proc_entry+0x1b3/0x3f0 [bonding]
>[ 58.655985] bond_netdev_event+0x433/0x970 [bonding]
>[ 58.656545] ? __module_text_address+0x13/0x140
>[ 58.657038] notifier_call_chain+0x90/0x160
>[ 58.657541] register_netdevice+0x9b3/0xd80
>[ 58.657999] ? alloc_netdev_mqs+0x854/0xc10
>[ 58.658476] ? netdev_change_features+0xa0/0xa0
>[ 58.663592] ? rtnl_create_link+0x2ed/0xad0
>[ 58.664049] bond_newlink+0x2a/0x60 [bonding]
>[ 58.664529] __rtnl_newlink+0xb9f/0x11b0
>[ 58.665014] ? rtnl_link_unregister+0x230/0x230
>[ ... ]
>
>Fixes: 0b680e753724 ("[PATCH] bonding: Add priv_flag to avoid event mishandling")
>Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com>
>---
>
>v2 -> v3 :
> - This patch is not changed
>v1 -> v2 :
> - Do not add a new priv_flag.
>
> drivers/net/bonding/bond_main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 931d9d935686..0db12fcfc953 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1816,7 +1816,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> slave_disable_netpoll(new_slave);
>
> err_close:
>- slave_dev->priv_flags &= ~IFF_BONDING;
>+ if (!netif_is_bond_master(slave_dev))
>+ slave_dev->priv_flags &= ~IFF_BONDING;
> dev_close(slave_dev);
>
> err_restore_mac:
>@@ -2017,7 +2018,8 @@ static int __bond_release_one(struct net_device *bond_dev,
> else
> dev_set_mtu(slave_dev, slave->original_mtu);
>
>- slave_dev->priv_flags &= ~IFF_BONDING;
>+ if (!netif_is_bond_master(slave_dev))
>+ slave_dev->priv_flags &= ~IFF_BONDING;
>
> bond_free_slave(slave);
>
>--
>2.17.1
>
^ permalink raw reply
* Re: [PATCH] dt-bindings: net: Correct the documentation of KSZ9021 skew values
From: David Miller @ 2019-09-16 14:14 UTC (permalink / raw)
To: james.byrne; +Cc: robh+dt, mark.rutland, netdev, devicetree
In-Reply-To: <0102016d2b84f180-bd396cb9-16cf-4472-b718-7a4d2d8d8017-000000@eu-west-1.amazonses.com>
From: James Byrne <james.byrne@origamienergy.com>
Date: Fri, 13 Sep 2019 16:46:35 +0000
> The documentation of skew values for the KSZ9021 PHY was misleading
> because the driver implementation followed the erroneous information
> given in the original KSZ9021 datasheet before it was corrected in
> revision 1.2 (Feb 2014). It is probably too late to correct the driver
> now because of the many existing device trees, so instead this just
> corrects the documentation to explain that what you actually get is not
> what you might think when looking at the device tree.
>
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
What tree should this go into?
^ permalink raw reply
* Re: [PATCH bpf] bpf: respect CAP_IPC_LOCK in RLIMIT_MEMLOCK check
From: Christian Barcenas @ 2019-09-16 14:09 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, netdev
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, bpf
In-Reply-To: <678ba696-4b20-5f06-7c4f-ec68a9229620@iogearbox.net>
> On 9/11/19 8:18 PM, Christian Barcenas wrote:
>> A process can lock memory addresses into physical RAM explicitly
>> (via mlock, mlockall, shmctl, etc.) or implicitly (via VFIO,
>> perf ring-buffers, bpf maps, etc.), subject to RLIMIT_MEMLOCK limits.
>>
>> CAP_IPC_LOCK allows a process to exceed these limits, and throughout
>> the kernel this capability is checked before allowing/denying an attempt
>> to lock memory regions into RAM.
>>
>> Because bpf locks its programs and maps into RAM, it should respect
>> CAP_IPC_LOCK. Previously, bpf would return EPERM when RLIMIT_MEMLOCK was
>> exceeded by a privileged process, which is contrary to documented
>> RLIMIT_MEMLOCK+CAP_IPC_LOCK behavior.
>
> Do you have a link/pointer where this is /clearly/ documented?
I admit that after submitting this patch, I did re-think the description
and thought maybe I should have described the CAP_IPC_LOCK behavior as
"expected" rather than "documented". :)
> ... but my best guess is you are referring to `man 2 mlock`:
>
> Limits and permissions
>
> In Linux 2.6.8 and earlier, a process must be privileged
> (CAP_IPC_LOCK)
> in order to lock memory and the RLIMIT_MEMLOCK soft resource
> limit defines
> a limit on how much memory the process may lock.
>
> Since Linux 2.6.9, no limits are placed on the amount of
> memory that a
> privileged process can lock and the RLIMIT_MEMLOCK soft resource
> limit
> instead defines a limit on how much memory an unprivileged
> process may lock.
Yes; this is what I was referring to by "documented
RLIMIT_MEMLOCK+CAP_IPC_LOCK behavior."
Unfortunately - AFAICT - this is the most explicit documentation about
CAP_IPC_LOCK's permission set, but it is incomplete.
I believe it can be understood from other references to RLIMIT and
CAP_IPC_LOCK throughout the kernel that "locking memory" refers not only
to mlock/shmctl syscalls, but also to other code sites where /physical/
memory addresses are allocated for userspace.
After identifying RLIMIT_MEMLOCK checks with
git grep -C3 '[^(get|set)]rlimit(RLIMIT_MEMLOCK'
we find that RLIMIT_MEMLOCK is bypassed - if CAP_IPC_LOCK is held - in
many locations that have nothing to do with the mlock or shm family of
syscalls. From what I can tell, every time RLIMIT_MEMLOCK is referenced
there is a neighboring check to CAP_IPC_LOCK that bypasses the rlimit,
or in some cases memory accounting entirely!
bpf() is currently the only exception to the above, ie. as far as I can
tell it is the only code that enforces RLIMIT_MEMLOCK but does not honor
CAP_IPC_LOCK.
Selected examples follow:
In net/core/skbuff.c:
if (capable(CAP_IPC_LOCK) || !size)
return 0;
num_pg = (size >> PAGE_SHIFT) + 2; /* worst case */
max_pg = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
user = mmp->user ? : current_user();
do {
old_pg = atomic_long_read(&user->locked_vm);
new_pg = old_pg + num_pg;
if (new_pg > max_pg)
return -ENOBUFS;
} while (atomic_long_cmpxchg(&user->locked_vm, old_pg, new_pg) !=
old_pg);
In net/xdp/xdp_umem.c:
if (capable(CAP_IPC_LOCK))
return 0;
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
umem->user = get_uid(current_user());
do {
old_npgs = atomic_long_read(&umem->user->locked_vm);
new_npgs = old_npgs + umem->npgs;
if (new_npgs > lock_limit) {
free_uid(umem->user);
umem->user = NULL;
return -ENOBUFS;
}
} while (atomic_long_cmpxchg(&umem->user->locked_vm, old_npgs,
new_npgs) != old_npgs);
return 0;
In arch/x86/kvm/svm.c:
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
if (locked > lock_limit && !capable(CAP_IPC_LOCK)) {
pr_err("SEV: %lu locked pages exceed the lock limit of
%lu.\n", locked, lock_limit);
return NULL;
}
In drivers/infiniband/core/umem.c (and other sites in Infiniband code):
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
new_pinned = atomic64_add_return(npages, &mm->pinned_vm);
if (new_pinned > lock_limit && !capable(CAP_IPC_LOCK)) {
atomic64_sub(npages, &mm->pinned_vm);
ret = -ENOMEM;
goto out;
}
In drivers/vfio/vfio_iommu_type1.c, albeit in an indirect way:
struct vfio_dma {
bool lock_cap; /* capable(CAP_IPC_LOCK) */
};
// ...
for (vaddr += PAGE_SIZE, iova += PAGE_SIZE; pinned < npage;
pinned++, vaddr += PAGE_SIZE, iova += PAGE_SIZE) {
// ...
if (!rsvd && !vfio_find_vpfn(dma, iova)) {
if (!dma->lock_cap &&
current->mm->locked_vm + lock_acct + 1 > limit) {
put_pfn(pfn, dma->prot);
pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
__func__, limit << PAGE_SHIFT);
ret = -ENOMEM;
goto unpin_out;
}
lock_acct++;
}
}
Best,
Christian
^ permalink raw reply
* Re: pull-request: bpf-next 2019-09-16
From: David Miller @ 2019-09-16 14:02 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev, bpf
In-Reply-To: <20190916102630.14491-1-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 16 Sep 2019 12:26:30 +0200
> The following pull-request contains BPF updates for your *net-next* tree.
Pulled and build testing (in Denmark!), thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 0/3] mlxsw: spectrum_buffers: Add the ability to query the CPU port's shared buffer
From: David Miller @ 2019-09-16 13:59 UTC (permalink / raw)
To: shalomt; +Cc: idosch, netdev, jiri, mlxsw, idosch
In-Reply-To: <95297977-0757-68c2-77f3-960056050fb3@mellanox.com>
From: Shalom Toledo <shalomt@mellanox.com>
Date: Mon, 16 Sep 2019 08:14:47 +0000
> I have v3 with all the fixes Jiri commented. Can I send it? Or should I wait
> until net-next will be open again?
Just send it, thanks for asking.
^ permalink raw reply
* Re: net: phy: micrel KSZ9031 ifdown ifup issue
From: Paul Thomas @ 2019-09-16 13:54 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190914145443.GE27922@lunn.ch>
Hi Andrew,
I did some more investigation, and what seems to be happening is the
device get's stuck in auto-negotiation. I looked at this using
phytool:
https://github.com/wkz/phytool
When it is in the good state I see that reg 0x01 is 0x796d where bit
1.2 reports 'Link is up' and bit 1.5 reports 'Auto-negotiation process
complete'. However, once I get to the bad state (it may take several
tries of ifdown, ifup to get there) then reg 0x01 is 0x7649 reporting
'Link is down' and 'Auto-negotiation process not completed'. This can
be fixed by resetting the phy './phytool write eth0/3/0 0x9140'
So, I guess that means the driver is doing what it is supposed to?
Could we add quirk or something to reset the phy again from the driver
if auto-negotiation doesn't complete with x seconds?
> Are you using interrupts, or polling? If interrupts, try polling?
> Seems unlikely, but you could be missing an interrupt.
It must be polling, the interrupt from the PHY is run in the
schematic, but it is not used in the hw or device-tree configuration.
>
> There is a fix from Antoine Tenart which suggests asym pause can be an
> issue? What pause setup are you using? But this is a known issue,
> which 5.2 should have the fix for.
Yes, this kernel includes this asym pause workaround. Reg 4.11:10 is 0
# ./phytool read eth0/3/4
0x01e1
This is the last little Ethernet issue that we are having, it would be
nice if we could find a solution.
thanks,
Paul
^ 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