* Re: [PATCH v2 bpf-next] tools/bpf: add missing strings.h include
From: Alexei Starovoitov @ 2019-02-08 2:21 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: acme, andrii.nakryiko, yhs, songliubraving, ast, kafai, netdev,
kernel-team
In-Reply-To: <20190207192924.2280663-1-andriin@fb.com>
On Thu, Feb 07, 2019 at 11:29:24AM -0800, Andrii Nakryiko wrote:
> Few files in libbpf are using bzero() function (defined in strings.h header), but
> don't include corresponding header. When libbpf is added as a dependency to pahole,
> this undeterministically causes warnings on some machines:
>
> bpf.c:225:2: warning: implicit declaration of function ‘bzero’ [-Wimplicit-function-declaration]
> bzero(&attr, sizeof(attr));
> ^~~~~
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Applied, Thanks
^ permalink raw reply
* Re: [PATCH 1/1] net: dsa: b53: Fix for failure when irq is not defined in dt
From: David Miller @ 2019-02-08 2:19 UTC (permalink / raw)
To: arun.parameswaran
Cc: f.fainelli, andrew, vivien.didelot, bcm-kernel-feedback-list,
netdev, linux-kernel
In-Reply-To: <20190208000118.9268-1-arun.parameswaran@broadcom.com>
From: Arun Parameswaran <arun.parameswaran@broadcom.com>
Date: Thu, 7 Feb 2019 16:01:18 -0800
> Fixes the issues with non BCM58XX chips in the b53 driver
> failing, when the irq is not specified in the device tree.
>
> Removed the check for BCM58XX in b53_srab_prepare_irq(),
> so the 'port->irq' will be set to '-EXIO' if the irq is not
> specified in the device tree.
>
> Fixes: 16994374a6fc ("net: dsa: b53: Make SRAB driver manage port interrupts")
> Fixes: b2ddc48a81b5 ("net: dsa: b53: Do not fail when IRQ are not initialized")
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: phy: let genphy_c45_read_link manage the devices to check
From: David Miller @ 2019-02-08 2:17 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, linux, netdev
In-Reply-To: <7c00df20-b3e1-7ea9-2adb-004e6291d52c@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Thu, 7 Feb 2019 21:41:46 +0100
> Let genphy_c45_read_link manage the devices to check, this removes
> overhead from callers. Add C22EXT to the list of excluded devices
> because it doesn't implement the status register. According to the
> 802.3 clause 45 spec registers 29.0 - 29.4 are reserved.
>
> At the moment we have very few clause 45 PHY drivers, so we are
> lacking experience whether other drivers will have to exclude further
> devices, or may need to check PHY XS. If we should figure out that
> list of devices to check needs to be configurable, I think best will
> be to add a device list member to struct phy_driver.
>
> v2:
> - adjusted commit message
> - exclude also device C22EXT from link checking
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: fixed-phy: Add fixed_phy_register_with_gpiod() API
From: David Miller @ 2019-02-08 2:15 UTC (permalink / raw)
To: mdf; +Cc: netdev, andrew, f.fainelli, hkallweit1, linux-kernel
In-Reply-To: <20190207201455.25071-1-mdf@kernel.org>
From: Moritz Fischer <mdf@kernel.org>
Date: Thu, 7 Feb 2019 12:14:55 -0800
> Add fixed_phy_register_with_gpiod() API. It lets users create a
> fixed_phy instance that uses a GPIO descriptor which was obtained
> externally e.g. through platform data.
> This enables platform devices (non-DT based) to use GPIOs for link
> status.
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
This doesn't apply cleanly to net-next, please respin.
^ permalink raw reply
* Re: [PATCH net-next v2 0/6] Add comphy support for Armada 38x
From: David Miller @ 2019-02-08 2:10 UTC (permalink / raw)
To: linux
Cc: andrew, gregory.clement, jason, kishon, sebastian.hesselbarth,
thomas.petazzoni, devicetree, linux-arm-kernel, mark.rutland,
netdev, robh+dt
In-Reply-To: <20190207161825.ueinmyf6ygjiqzzy@shell.armlinux.org.uk>
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Thu, 7 Feb 2019 16:18:25 +0000
> This series adds support for the comphy for Armada 38x, which allows
> these SoCs to use 2500BASE-X mode with appropriate SFP modules.
>
> Tested on SolidRun Clearfog after updating for the 5.0 merge window
> changes.
Series applied, thanks Russell.
^ permalink raw reply
* Re: [PATCH net-next 0/7] net/smc: patches 2019-02-07
From: David Miller @ 2019-02-08 2:06 UTC (permalink / raw)
To: ubraun; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl
In-Reply-To: <20190207145620.92811-1-ubraun@linux.ibm.com>
From: Ursula Braun <ubraun@linux.ibm.com>
Date: Thu, 7 Feb 2019 15:56:13 +0100
> here are patches for SMC:
> * patches 1, 3, and 6 are cleanups without functional change
> * patch 2 postpones closing of internal clcsock
> * patches 4 and 5 improve link group creation locking
> * patch 7 restores AF_SMC as diag_family field
Series applied, thank you.
^ permalink raw reply
* [PATCH net-next] net: dsa: use struct_size() in devm_kzalloc()
From: Gustavo A. R. Silva @ 2019-02-08 1:16 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller
Cc: netdev, linux-kernel, Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
size = sizeof(struct foo) + count * sizeof(struct boo);
instance = alloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
instance = alloc(struct_size(instance, entry, count), GFP_KERNEL)
Notice that, in this case, variable size is not necessary, hence it is
removed.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/dsa/dsa2.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a1917025e155..8c431e0f3627 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -767,11 +767,10 @@ static int dsa_switch_probe(struct dsa_switch *ds)
struct dsa_switch *dsa_switch_alloc(struct device *dev, size_t n)
{
- size_t size = sizeof(struct dsa_switch) + n * sizeof(struct dsa_port);
struct dsa_switch *ds;
int i;
- ds = devm_kzalloc(dev, size, GFP_KERNEL);
+ ds = devm_kzalloc(dev, struct_size(ds, ports, n), GFP_KERNEL);
if (!ds)
return NULL;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next] mpls_iptunnel: use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08 1:10 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
instance = alloc(sizeof(struct foo) + count * sizeof(struct boo));
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
instance = alloc(struct_size(instance, entry, count));
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/mpls/mpls_iptunnel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index 94f53a9b7d1a..dda8930f20e7 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -183,8 +183,8 @@ static int mpls_build_state(struct nlattr *nla,
&n_labels, NULL, extack))
return -EINVAL;
- newts = lwtunnel_state_alloc(sizeof(*tun_encap_info) +
- n_labels * sizeof(u32));
+ newts = lwtunnel_state_alloc(struct_size(tun_encap_info, label,
+ n_labels));
if (!newts)
return -ENOMEM;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next] net/sched: use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08 1:02 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller
Cc: netdev, linux-kernel, Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
size = sizeof(struct foo) + count * sizeof(struct boo);
instance = alloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
size = struct_size(instance, entry, count);
instance = alloc(size, GFP_KERNEL)
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/sched/act_pedit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 2b372a06b432..3663d3b615a4 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -406,7 +406,7 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_t t;
int s;
- s = sizeof(*opt) + p->tcfp_nkeys * sizeof(struct tc_pedit_key);
+ s = struct_size(opt, keys, p->tcfp_nkeys);
/* netlink spinlocks held above us - must use ATOMIC */
opt = kzalloc(s, GFP_ATOMIC);
--
2.20.1
^ permalink raw reply related
* [PATCH][next] Bluetooth: hci_event: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08 0:40 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel, Gustavo A. R. Silva
Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes, in particular in the
context in which this code is being used.
So, change the following form:
sizeof(*ev) + ev->num_hndl * sizeof(struct hci_comp_pkts_info)
to :
struct_size(ev, handles, ev->num_hndl)
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/bluetooth/hci_event.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ac2826ce162b..609fd6871c5a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3556,8 +3556,8 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *skb)
return;
}
- if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
- ev->num_hndl * sizeof(struct hci_comp_pkts_info)) {
+ if (skb->len < sizeof(*ev) ||
+ skb->len < struct_size(ev, handles, ev->num_hndl)) {
BT_DBG("%s bad parameters", hdev->name);
return;
}
@@ -3644,8 +3644,8 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, struct sk_buff *skb)
return;
}
- if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
- ev->num_hndl * sizeof(struct hci_comp_blocks_info)) {
+ if (skb->len < sizeof(*ev) ||
+ skb->len < struct_size(ev, handles, ev->num_hndl)) {
BT_DBG("%s bad parameters", hdev->name);
return;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net-next] bridge: use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08 0:58 UTC (permalink / raw)
To: Roopa Prabhu, Nikolay Aleksandrov, David S. Miller
Cc: bridge, netdev, linux-kernel, Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
size = sizeof(struct foo) + count * sizeof(struct boo);
instance = alloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
size = struct_size(instance, entry, count);
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/bridge/br_multicast.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 1fb885a33c66..4a048fd1cbea 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1018,8 +1018,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
if (!nsrcs)
return -EINVAL;
- grec_len = sizeof(*grec) +
- sizeof(struct in6_addr) * ntohs(*nsrcs);
+ grec_len = struct_size(grec, grec_src, ntohs(*nsrcs));
if (!ipv6_mc_may_pull(skb, len + grec_len))
return -EINVAL;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next] netfilter: xt_recent: Use struct_size() in kvzalloc()
From: Gustavo A. R. Silva @ 2019-02-08 0:56 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller
Cc: netfilter-devel, coreteam, netdev, linux-kernel,
Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
void *entry[];
};
size = sizeof(struct foo) + count * sizeof(void *);
instance = alloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
size = struct_size(instance, entry, count);
instance = alloc(size, GFP_KERNEL)
Notice that, in this case, variable sz is not necessary, hence it is
removed.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/netfilter/xt_recent.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index f44de4bc2100..1664d2ec8b2f 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -337,7 +337,6 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
unsigned int nstamp_mask;
unsigned int i;
int ret = -EINVAL;
- size_t sz;
net_get_random_once(&hash_rnd, sizeof(hash_rnd));
@@ -387,8 +386,7 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
goto out;
}
- sz = sizeof(*t) + sizeof(t->iphash[0]) * ip_list_hash_size;
- t = kvzalloc(sz, GFP_KERNEL);
+ t = kvzalloc(struct_size(t, iphash, ip_list_hash_size), GFP_KERNEL);
if (t == NULL) {
ret = -ENOMEM;
goto out;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next] ipvs: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08 0:44 UTC (permalink / raw)
To: Wensong Zhang, Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, David S. Miller
Cc: netdev, lvs-devel, netfilter-devel, coreteam, linux-kernel,
Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
size = sizeof(struct foo) + count * sizeof(struct boo);
instance = alloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
size = struct_size(instance, entry, count);
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/netfilter/ipvs/ip_vs_ctl.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 7d6318664eb2..bcd9112f47d9 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2734,8 +2734,7 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
int size;
get = (struct ip_vs_get_services *)arg;
- size = sizeof(*get) +
- sizeof(struct ip_vs_service_entry) * get->num_services;
+ size = struct_size(get, entrytable, get->num_services);
if (*len != size) {
pr_err("length: %u != %u\n", *len, size);
ret = -EINVAL;
@@ -2776,8 +2775,7 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
int size;
get = (struct ip_vs_get_dests *)arg;
- size = sizeof(*get) +
- sizeof(struct ip_vs_dest_entry) * get->num_dests;
+ size = struct_size(get, entrytable, get->num_dests);
if (*len != size) {
pr_err("length: %u != %u\n", *len, size);
ret = -EINVAL;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] mlxsw: spectrum_router: Offload blackhole routes
From: David Ahern @ 2019-02-08 0:40 UTC (permalink / raw)
To: Ido Schimmel, netdev@vger.kernel.org
Cc: davem@davemloft.net, Jiri Pirko, Alexander Petrovskiy, mlxsw
In-Reply-To: <20190206194140.18606-2-idosch@mellanox.com>
On 2/6/19 11:42 AM, Ido Schimmel wrote:
> Create a new FIB entry type for blackhole routes and set it in case the
> type of the notified route is 'RTN_BLACKHOLE'.
>
> Program such routes with a discard action and mark them as offloaded
> since the device is dropping the packets instead of the kernel.
>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
> .../ethernet/mellanox/mlxsw/spectrum_router.c | 27 +++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
One of the feature requests from the FRR team (and a feature I have
implemented) is a blackhole nexthop. The idea is that prefixes are
installed pointing to nexthop id N. That nexthop definition can be
atomically updated to go between a device / gateway and a blackhole.
[ prefix ] --> [ nhid 1 ] --> [ dev1 / gateway1 ]
[ prefix ] --> [ nhid 1 ] --> [ blackhole ]
[ prefix ] --> [ nhid 1 ] --> [ dev2 / gateway2 ]
Do you see this working ok with mlxsw without having to update the
prefix entries (which can be numerous) directly?
^ permalink raw reply
* [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08 0:28 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel, Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
size = sizeof(struct foo) + count * sizeof(struct boo);
instance = alloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
size = struct_size(instance, entry, count);
instance = alloc(size, GFP_KERNEL)
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/bluetooth/a2mp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index 58fc6333d412..5f918ea18b5a 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -174,7 +174,7 @@ static int a2mp_discover_req(struct amp_mgr *mgr, struct sk_buff *skb,
num_ctrl++;
}
- len = num_ctrl * sizeof(struct a2mp_cl) + sizeof(*rsp);
+ len = struct_size(rsp, cl, num_ctrl);
rsp = kmalloc(len, GFP_ATOMIC);
if (!rsp) {
read_unlock(&hci_dev_list_lock);
--
2.20.1
^ permalink raw reply related
* Re: [iproute PATCH] ip-link: Fix listing of alias interfaces
From: Stephen Hemminger @ 2019-02-08 0:24 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev, Roopa Prabhu
In-Reply-To: <20190207130527.9439-1-phil@nwl.cc>
On Thu, 7 Feb 2019 14:05:27 +0100
Phil Sutter <phil@nwl.cc> wrote:
> Commit 50b9950dd9011 ("link dump filter") accidentally broke listing of
> links in the old alias interface notation:
>
> | % ip link show eth0:1
> | RTNETLINK answers: No such device
> | Cannot send link get request: No such device
>
> Prior to the above commit, link lookup was performed via ifindex
> returned by if_nametoindex(). The latter uses SIOCGIFINDEX ioctl call
> which on kernel side causes the colon-suffix to be dropped before doing
> the interface lookup. Netlink API though doesn't care about that at all.
> To keep things backward compatible, mimick ioctl API behaviour and drop
> the colon-suffix prior to sending the RTM_GETLINK request.
>
> Fixes: 50b9950dd9011 ("link dump filter")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
What about mistaken usage where the text after the colon is not a number,
or has additional colon?
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2019-02-08 0:24 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List, Tonghao Zhang,
Pablo Neira Ayuso
[-- Attachment #1: Type: text/plain, Size: 8878 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
between commit:
218d05ce326f ("net/mlx5e: Don't overwrite pedit action when multiple pedit used")
from the net tree and commit:
c500c86b0c75 ("net/mlx5e: support for two independent packet edit actions")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index b5c1b039375a,85c5dd7fc2c7..000000000000
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@@ -1310,15 -1308,12 +1309,12 @@@ static int parse_tunnel_attr(struct mlx
outer_headers);
void *headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value,
outer_headers);
-
- struct flow_dissector_key_control *enc_control =
- skb_flow_dissector_target(f->dissector,
- FLOW_DISSECTOR_KEY_ENC_CONTROL,
- f->key);
- int err = 0;
+ struct flow_rule *rule = tc_cls_flower_offload_flow_rule(f);
+ struct flow_match_control enc_control;
+ int err;
err = mlx5e_tc_tun_parse(filter_dev, priv, spec, f,
- headers_c, headers_v);
+ headers_c, headers_v, match_level);
if (err) {
NL_SET_ERR_MSG_MOD(extack,
"failed to parse tunnel attributes");
@@@ -1466,19 -1454,17 +1455,17 @@@ static int __parse_cls_flower(struct ml
return -EOPNOTSUPP;
}
- if ((dissector_uses_key(f->dissector,
- FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS) ||
- dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_ENC_KEYID) ||
- dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_ENC_PORTS)) &&
- dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_ENC_CONTROL)) {
- struct flow_dissector_key_control *key =
- skb_flow_dissector_target(f->dissector,
- FLOW_DISSECTOR_KEY_ENC_CONTROL,
- f->key);
- switch (key->addr_type) {
+ if ((flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS) ||
+ flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_KEYID) ||
+ flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_PORTS)) &&
+ flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_CONTROL)) {
+ struct flow_match_control match;
+
+ flow_rule_match_enc_control(rule, &match);
+ switch (match.key->addr_type) {
case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
- if (parse_tunnel_attr(priv, spec, f, filter_dev))
+ if (parse_tunnel_attr(priv, spec, f, filter_dev, tunnel_match_level))
return -EOPNOTSUPP;
break;
default:
@@@ -1937,12 -1880,11 +1883,11 @@@ static struct mlx5_fields fields[] =
OFFLOAD(UDP_DPORT, 2, udp.dest, 0),
};
-/* On input attr->num_mod_hdr_actions tells how many HW actions can be parsed at
- * max from the SW pedit action. On success, it says how many HW actions were
- * actually parsed.
+/* On input attr->max_mod_hdr_actions tells how many HW actions can be parsed at
+ * max from the SW pedit action. On success, attr->num_mod_hdr_actions
+ * says how many HW actions were actually parsed.
*/
- static int offload_pedit_fields(struct pedit_headers *masks,
- struct pedit_headers *vals,
+ static int offload_pedit_fields(struct pedit_headers_action *hdrs,
struct mlx5e_tc_flow_parse_attr *parse_attr,
struct netlink_ext_ack *extack)
{
@@@ -1957,17 -1899,15 +1902,17 @@@
__be16 mask_be16;
void *action;
- set_masks = &masks[TCA_PEDIT_KEY_EX_CMD_SET];
- add_masks = &masks[TCA_PEDIT_KEY_EX_CMD_ADD];
- set_vals = &vals[TCA_PEDIT_KEY_EX_CMD_SET];
- add_vals = &vals[TCA_PEDIT_KEY_EX_CMD_ADD];
+ set_masks = &hdrs[0].masks;
+ add_masks = &hdrs[1].masks;
+ set_vals = &hdrs[0].vals;
+ add_vals = &hdrs[1].vals;
action_size = MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto);
- action = parse_attr->mod_hdr_actions;
- max_actions = parse_attr->num_mod_hdr_actions;
- nactions = 0;
+ action = parse_attr->mod_hdr_actions +
+ parse_attr->num_mod_hdr_actions * action_size;
+
+ max_actions = parse_attr->max_mod_hdr_actions;
+ nactions = parse_attr->num_mod_hdr_actions;
for (i = 0; i < ARRAY_SIZE(fields); i++) {
f = &fields[i];
@@@ -2085,52 -2027,53 +2032,55 @@@ static int alloc_mod_hdr_actions(struc
static const struct pedit_headers zero_masks = {};
static int parse_tc_pedit_action(struct mlx5e_priv *priv,
- const struct tc_action *a, int namespace,
+ const struct flow_action_entry *act, int namespace,
struct mlx5e_tc_flow_parse_attr *parse_attr,
+ struct pedit_headers_action *hdrs,
struct netlink_ext_ack *extack)
{
- struct pedit_headers masks[__PEDIT_CMD_MAX], vals[__PEDIT_CMD_MAX], *cmd_masks;
- int nkeys, i, err = -EOPNOTSUPP;
+ u8 cmd = (act->id == FLOW_ACTION_MANGLE) ? 0 : 1;
+ int err = -EOPNOTSUPP;
u32 mask, val, offset;
- u8 cmd, htype;
+ u8 htype;
- nkeys = tcf_pedit_nkeys(a);
+ htype = act->mangle.htype;
+ err = -EOPNOTSUPP; /* can't be all optimistic */
- memset(masks, 0, sizeof(struct pedit_headers) * __PEDIT_CMD_MAX);
- memset(vals, 0, sizeof(struct pedit_headers) * __PEDIT_CMD_MAX);
+ if (htype == FLOW_ACT_MANGLE_UNSPEC) {
+ NL_SET_ERR_MSG_MOD(extack, "legacy pedit isn't offloaded");
+ goto out_err;
+ }
- for (i = 0; i < nkeys; i++) {
- htype = tcf_pedit_htype(a, i);
- cmd = tcf_pedit_cmd(a, i);
- err = -EOPNOTSUPP; /* can't be all optimistic */
+ mask = act->mangle.mask;
+ val = act->mangle.val;
+ offset = act->mangle.offset;
- if (htype == TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK) {
- NL_SET_ERR_MSG_MOD(extack,
- "legacy pedit isn't offloaded");
- goto out_err;
- }
+ err = set_pedit_val(htype, ~mask, val, offset, &hdrs[cmd]);
+ if (err)
+ goto out_err;
- if (cmd != TCA_PEDIT_KEY_EX_CMD_SET && cmd != TCA_PEDIT_KEY_EX_CMD_ADD) {
- NL_SET_ERR_MSG_MOD(extack, "pedit cmd isn't offloaded");
- goto out_err;
- }
+ hdrs[cmd].pedits++;
- mask = tcf_pedit_mask(a, i);
- val = tcf_pedit_val(a, i);
- offset = tcf_pedit_offset(a, i);
+ return 0;
+ out_err:
+ return err;
+ }
- err = set_pedit_val(htype, ~mask, val, offset, &masks[cmd], &vals[cmd]);
- if (err)
- goto out_err;
- }
+ static int alloc_tc_pedit_action(struct mlx5e_priv *priv, int namespace,
+ struct mlx5e_tc_flow_parse_attr *parse_attr,
+ struct pedit_headers_action *hdrs,
+ struct netlink_ext_ack *extack)
+ {
+ struct pedit_headers *cmd_masks;
+ int err;
+ u8 cmd;
- err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
- if (err)
- goto out_err;
+ if (!parse_attr->mod_hdr_actions) {
- err = alloc_mod_hdr_actions(priv, a, namespace, parse_attr);
++ err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
+ if (err)
+ goto out_err;
+ }
- err = offload_pedit_fields(masks, vals, parse_attr, extack);
+ err = offload_pedit_fields(hdrs, parse_attr, extack);
if (err < 0)
goto out_dealloc_parsed_actions;
@@@ -2185,22 -2128,17 +2135,22 @@@ static bool csum_offload_supported(stru
}
static bool modify_header_match_supported(struct mlx5_flow_spec *spec,
- struct tcf_exts *exts,
+ struct flow_action *flow_action,
+ u32 actions,
struct netlink_ext_ack *extack)
{
- const struct tc_action *a;
+ const struct flow_action_entry *act;
bool modify_ip_header;
u8 htype, ip_proto;
void *headers_v;
u16 ethertype;
- int nkeys, i;
+ int i;
- headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value, outer_headers);
+ if (actions & MLX5_FLOW_CONTEXT_ACTION_DECAP)
+ headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value, inner_headers);
+ else
+ headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value, outer_headers);
+
ethertype = MLX5_GET(fte_match_set_lyr_2_4, headers_v, ethertype);
/* for non-IP we only re-write MACs, so we're okay */
@@@ -2256,8 -2190,9 +2202,9 @@@ static bool actions_match_supported(str
return false;
if (actions & MLX5_FLOW_CONTEXT_ACTION_MOD_HDR)
- return modify_header_match_supported(&parse_attr->spec, exts,
+ return modify_header_match_supported(&parse_attr->spec,
+ flow_action,
- extack);
+ actions, extack);
return true;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] net: dsa: b53: Fix for failure when irq is not defined in dt
From: Florian Fainelli @ 2019-02-08 0:07 UTC (permalink / raw)
To: Arun Parameswaran, Andrew Lunn, Vivien Didelot, David S . Miller
Cc: bcm-kernel-feedback-list, netdev, linux-kernel
In-Reply-To: <20190208000118.9268-1-arun.parameswaran@broadcom.com>
On 2/7/19 4:01 PM, Arun Parameswaran wrote:
> Fixes the issues with non BCM58XX chips in the b53 driver
> failing, when the irq is not specified in the device tree.
>
> Removed the check for BCM58XX in b53_srab_prepare_irq(),
> so the 'port->irq' will be set to '-EXIO' if the irq is not
> specified in the device tree.
>
> Fixes: 16994374a6fc ("net: dsa: b53: Make SRAB driver manage port interrupts")
> Fixes: b2ddc48a81b5 ("net: dsa: b53: Do not fail when IRQ are not initialized")
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks Arun!
> ---
> drivers/net/dsa/b53/b53_srab.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
> index 90f514252987..d9c56a779c08 100644
> --- a/drivers/net/dsa/b53/b53_srab.c
> +++ b/drivers/net/dsa/b53/b53_srab.c
> @@ -511,9 +511,6 @@ static void b53_srab_prepare_irq(struct platform_device *pdev)
> /* Clear all pending interrupts */
> writel(0xffffffff, priv->regs + B53_SRAB_INTR);
>
> - if (dev->pdata && dev->pdata->chip_id != BCM58XX_DEVICE_ID)
> - return;
> -
> for (i = 0; i < B53_N_PORTS; i++) {
> port = &priv->port_intrs[i];
>
>
--
Florian
^ permalink raw reply
* [PATCH 1/1] net: dsa: b53: Fix for failure when irq is not defined in dt
From: Arun Parameswaran @ 2019-02-08 0:01 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S . Miller
Cc: bcm-kernel-feedback-list, netdev, linux-kernel, Arun Parameswaran
Fixes the issues with non BCM58XX chips in the b53 driver
failing, when the irq is not specified in the device tree.
Removed the check for BCM58XX in b53_srab_prepare_irq(),
so the 'port->irq' will be set to '-EXIO' if the irq is not
specified in the device tree.
Fixes: 16994374a6fc ("net: dsa: b53: Make SRAB driver manage port interrupts")
Fixes: b2ddc48a81b5 ("net: dsa: b53: Do not fail when IRQ are not initialized")
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
---
drivers/net/dsa/b53/b53_srab.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
index 90f514252987..d9c56a779c08 100644
--- a/drivers/net/dsa/b53/b53_srab.c
+++ b/drivers/net/dsa/b53/b53_srab.c
@@ -511,9 +511,6 @@ static void b53_srab_prepare_irq(struct platform_device *pdev)
/* Clear all pending interrupts */
writel(0xffffffff, priv->regs + B53_SRAB_INTR);
- if (dev->pdata && dev->pdata->chip_id != BCM58XX_DEVICE_ID)
- return;
-
for (i = 0; i < B53_N_PORTS; i++) {
port = &priv->port_intrs[i];
--
2.17.1
^ permalink raw reply related
* RE: [PATCH v2 4/6] ethtool: support per-queue sub command --show-coalesce
From: Nunley, Nicholas D @ 2019-02-07 23:53 UTC (permalink / raw)
To: Michal Kubecek, netdev@vger.kernel.org
Cc: Kirsher, Jeffrey T, linville@tuxdriver.com, nhorman@redhat.com,
sassmann@redhat.com
In-Reply-To: <20190206132228.GB18410@unicorn.suse.cz>
> -----Original Message-----
> From: Michal Kubecek [mailto:mkubecek@suse.cz]
> Sent: Wednesday, February 6, 2019 5:22 AM
> To: netdev@vger.kernel.org
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; linville@tuxdriver.com;
> Nunley, Nicholas D <nicholas.d.nunley@intel.com>; nhorman@redhat.com;
> sassmann@redhat.com
> Subject: Re: [PATCH v2 4/6] ethtool: support per-queue sub command --
> show-coalesce
>
> On Tue, Feb 05, 2019 at 04:01:04PM -0800, Jeff Kirsher wrote:
> > diff --git a/ethtool.c b/ethtool.c
> > index 4dc725c..9a1b83b 100644
> > --- a/ethtool.c
> > +++ b/ethtool.c
> > @@ -1409,6 +1409,29 @@ static int dump_coalesce(const struct
> ethtool_coalesce *ecoal)
> > return 0;
> > }
> >
> > +void dump_per_queue_coalesce(struct ethtool_per_queue_op
> *per_queue_opt,
> > + __u32 *queue_mask)
> > +{
> > + char *addr;
> > + int i;
> > +
> > + addr = (char *)per_queue_opt + sizeof(*per_queue_opt);
> > + for (i = 0; i < __KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32);
> i++) {
> > + int queue = i * 32;
> > + __u32 mask = queue_mask[i];
> > +
> > + while (mask > 0) {
> > + if (mask & 0x1) {
> > + fprintf(stdout, "Queue: %d\n", queue);
> > + dump_coalesce((struct ethtool_coalesce
> *)addr);
> > + addr += sizeof(struct ethtool_coalesce);
> > + }
> > + mask = mask >> 1;
> > + queue++;
> > + }
> > + }
> > +}
> > +
> > struct feature_state {
> > u32 off_flags;
> > struct ethtool_gfeatures features;
>
> The casts and pointer arithmetic are a bit complicated. How about this?
>
> struct ethtool_coalesce *addr;
> ...
> addr = (struct ethtool_coalesce *)(per_queue_opt + 1); ...
> dump_coalesce(addr);
> addr++;
>
> Also passing n_queue down from do_perqueue() would prevent having to
> parse the whole bitmap even if we already know the NIC has only few
> queues.
Okay, that does make the pointer arithmetic a little more straightforward, so I'll change this. I'll also add the n_queue parameter to set_per_queue_coalesce() and dump_per_queue_coalesce() so we can exit early once we've found all the queues.
> > @@ -5245,7 +5268,8 @@ static const struct option {
> > { "--show-fec", 1, do_gfec, "Show FEC settings"},
> > { "--set-fec", 1, do_sfec, "Set FEC settings",
> > " [ encoding auto|off|rs|baser [...]]\n"},
> > - { "--set-perqueue-command", 1, do_perqueue, "Set per queue
> command",
> > + { "--set-perqueue-command", 1, do_perqueue, "Set per queue
> command. "
> > + "The supported sub commands include --show-coalesce",
> > " [queue_mask %x] SUB_COMMAND\n"},
> > { "-h|--help", 0, show_usage, "Show this help" },
> > { "--version", 0, do_version, "Show version number" }, @@ -5350,8
> > +5374,32 @@ static int find_max_num_queues(struct cmd_context *ctx)
> > echannels.combined_count);
> > }
> >
> > +static struct ethtool_per_queue_op *
> > +get_per_queue_coalesce(struct cmd_context *ctx, __u32 *queue_mask,
> > +int n_queues) {
> > + struct ethtool_per_queue_op *per_queue_opt;
> > +
> > + per_queue_opt = malloc(sizeof(*per_queue_opt) + n_queues *
> > + sizeof(struct ethtool_coalesce));
> > + if (!per_queue_opt)
> > + return NULL;
> > +
> > + memcpy(per_queue_opt->queue_mask, queue_mask,
> > + __KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32) *
> sizeof(__u32));
> > + per_queue_opt->cmd = ETHTOOL_PERQUEUE;
> > + per_queue_opt->sub_command = ETHTOOL_GCOALESCE;
> > + if (send_ioctl(ctx, per_queue_opt)) {
> > + free(per_queue_opt);
> > + perror("Cannot get device per queue parameters");
> > + return NULL;
> > + }
> > +
> > + return per_queue_opt;
> > +}
> > +
> > static int do_perqueue(struct cmd_context *ctx) {
> > + struct ethtool_per_queue_op *per_queue_opt;
> > __u32
> queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)] = {0};
> > int i, n_queues = 0;
> >
> > @@ -5390,7 +5438,19 @@ static int do_perqueue(struct cmd_context *ctx)
> > if (i < 0)
> > exit_bad_args();
> >
> > - /* no sub_command support yet */
> > + if (strstr(args[i].opts, "--show-coalesce") != NULL) {
>
> Comparing args[i].func to do_gcoalesce might be easier.
This is the one comment where I think it's better to leave the code as it is. To me is seems more confusing to match on a function pointer that we're never going to call. Unless there are more objections I'd rather keep it the way it is.
Otherwise, thanks for the review and all the comments. I'll work on making these changes and get an updated version submitted.
Nick
>
> > + per_queue_opt = get_per_queue_coalesce(ctx,
> queue_mask,
> > + n_queues);
> > + if (per_queue_opt == NULL) {
> > + perror("Cannot get device per queue parameters");
> > + return -EFAULT;
> > + }
> > + dump_per_queue_coalesce(per_queue_opt, queue_mask);
> > + free(per_queue_opt);
> > + } else {
> > + perror("The subcommand is not supported yet");
> > + return -EOPNOTSUPP;
> > + }
> >
> > return 0;
> > }
>
> Michal Kubecek
^ permalink raw reply
* Re: [PATCH net-next v2 07/10] net: phy: marvell10g: Add support for 2.5GBASET
From: Russell King - ARM Linux admin @ 2019-02-07 23:48 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-arm-kernel, Antoine Tenart,
thomas.petazzoni, gregory.clement, miquel.raynal, nadavh, stefanc,
mw
In-Reply-To: <20190207094939.27369-8-maxime.chevallier@bootlin.com>
On Thu, Feb 07, 2019 at 10:49:36AM +0100, Maxime Chevallier wrote:
> The Marvell Alaska family of PHYs supports 2.5GBaseT and 5GBaseT modes,
> as defined in the 802.3bz specification.
>
> When the link partner requests a 2.5GBASET link, the PHY will
> reconfigure it's MII interface to 2500BASEX.
>
> At 5G, the PHY will reconfigure it's interface to 5GBASE-R, but this
> mode isn't supported by any MAC for now.
>
> This was tested with :
> - The 88X3310, which is on the MacchiatoBin
Hi Maxime,
Looking deeper at this, I think we actually need an additional patch at
the beginning of your series.
The default AN advertisement in 7.32 is 0x1181 - which includes the
2.5G and 5G modes. We need to clear these bits, so that when the 10G
mode disabled via ethtool, we do not switch to 2.5G or 5G speed (both
of which are not currently reported as supported.) Such a patch needs
backporting to stable kernels.
> - The 88E2010, an Alaska PHY that has no fiber interfaces, and is
> limited to 5G maximum speed.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> V1 -> V2: Use a #define for the 88X3310 PHY ID, since it's reused in
> various places in the code. Rebased on Heiner Kallweit's patch
> introducing the phy_modify_mmd accessor.
>
> drivers/net/phy/marvell10g.c | 30 ++++++++++++++++++++++--------
> include/linux/marvell_phy.h | 1 +
> 2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 07df87b81369..581b4b6e31e9 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -238,6 +238,7 @@ static int mv3310_config_init(struct phy_device *phydev)
>
> /* Check that the PHY interface type is compatible */
> if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
> + phydev->interface != PHY_INTERFACE_MODE_2500BASEX &&
> phydev->interface != PHY_INTERFACE_MODE_XAUI &&
> phydev->interface != PHY_INTERFACE_MODE_RXAUI &&
> phydev->interface != PHY_INTERFACE_MODE_10GKR)
> @@ -307,8 +308,18 @@ static int mv3310_config_aneg(struct phy_device *phydev)
> else
> reg = 0;
>
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
> + phydev->advertising))
> + reg |= MDIO_AN_10GBT_CTRL_ADV2_5G;
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
> + phydev->advertising))
> + reg |= MDIO_AN_10GBT_CTRL_ADV5G;
> +
> ret = phy_modify_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
> - MDIO_AN_10GBT_CTRL_ADV10G, reg);
> + MDIO_AN_10GBT_CTRL_ADV10G |
> + MDIO_AN_10GBT_CTRL_ADV5G |
> + MDIO_AN_10GBT_CTRL_ADV2_5G, reg);
> +
> if (ret < 0)
> return ret;
> if (ret > 0)
> @@ -337,17 +348,20 @@ static int mv3310_aneg_done(struct phy_device *phydev)
> static void mv3310_update_interface(struct phy_device *phydev)
> {
> if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
> + phydev->interface == PHY_INTERFACE_MODE_2500BASEX ||
> phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
> /* The PHY automatically switches its serdes interface (and
> - * active PHYXS instance) between Cisco SGMII and 10GBase-KR
> - * modes according to the speed. Florian suggests setting
> - * phydev->interface to communicate this to the MAC. Only do
> - * this if we are already in either SGMII or 10GBase-KR mode.
> + * active PHYXS instance) between Cisco SGMII, 10GBase-KR and
> + * 2500BaseX modes according to the speed. Florian suggests
> + * setting phydev->interface to communicate this to the MAC.
> + * Only do this if we are already in one of the above modes.
> */
> if (phydev->speed == SPEED_10000)
> phydev->interface = PHY_INTERFACE_MODE_10GKR;
> + else if (phydev->speed == SPEED_2500)
> + phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
> else if (phydev->speed >= SPEED_10 &&
> - phydev->speed < SPEED_10000)
> + phydev->speed < SPEED_2500)
> phydev->interface = PHY_INTERFACE_MODE_SGMII;
> }
> }
> @@ -450,7 +464,7 @@ static int mv3310_read_status(struct phy_device *phydev)
>
> static struct phy_driver mv3310_drivers[] = {
> {
> - .phy_id = 0x002b09aa,
> + .phy_id = MARVELL_PHY_ID_88X3310,
> .phy_id_mask = MARVELL_PHY_ID_MASK,
> .name = "mv88x3310",
> .features = PHY_10GBIT_FEATURES,
> @@ -468,7 +482,7 @@ static struct phy_driver mv3310_drivers[] = {
> module_phy_driver(mv3310_drivers);
>
> static struct mdio_device_id __maybe_unused mv3310_tbl[] = {
> - { 0x002b09aa, MARVELL_PHY_ID_MASK },
> + { MARVELL_PHY_ID_88X3310, MARVELL_PHY_ID_MASK },
> { },
> };
> MODULE_DEVICE_TABLE(mdio, mv3310_tbl);
> diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
> index 1eb6f244588d..5851d68d828a 100644
> --- a/include/linux/marvell_phy.h
> +++ b/include/linux/marvell_phy.h
> @@ -20,6 +20,7 @@
> #define MARVELL_PHY_ID_88E1540 0x01410eb0
> #define MARVELL_PHY_ID_88E1545 0x01410ea0
> #define MARVELL_PHY_ID_88E3016 0x01410e60
> +#define MARVELL_PHY_ID_88X3310 0x002b09aa
>
> /* The MV88e6390 Ethernet switch contains embedded PHYs. These PHYs do
> * not have a model ID. So the switch driver traps reads to the ID2
> --
> 2.19.2
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* RE: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue settings
From: Nunley, Nicholas D @ 2019-02-07 23:43 UTC (permalink / raw)
To: Michal Kubecek, netdev@vger.kernel.org
Cc: Kirsher, Jeffrey T, linville@tuxdriver.com, nhorman@redhat.com,
sassmann@redhat.com
In-Reply-To: <20190206124307.GA18410@unicorn.suse.cz>
> -----Original Message-----
> From: Michal Kubecek [mailto:mkubecek@suse.cz]
> Sent: Wednesday, February 6, 2019 4:43 AM
> To: netdev@vger.kernel.org
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; linville@tuxdriver.com;
> Nunley, Nicholas D <nicholas.d.nunley@intel.com>; nhorman@redhat.com;
> sassmann@redhat.com
> Subject: Re: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue
> settings
>
> On Tue, Feb 05, 2019 at 04:01:03PM -0800, Jeff Kirsher wrote:
> > +static int do_perqueue(struct cmd_context *ctx) {
> > + __u32
> queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)] = {0};
> > + int i, n_queues = 0;
> > +
> > + if (ctx->argc == 0)
> > + exit_bad_args();
> > +
> > + /*
> > + * The sub commands will be applied to
> > + * all queues if no queue_mask set
> > + */
> > + if (strncmp(*ctx->argp, "queue_mask", 10)) {
>
> This would match any string starting with "queue_mask", is it intended?
No, I'll fix this. I don't know that there are any use cases where this distinction would matter, but then again there's no reason to have it match this way.
>
> > + n_queues = find_max_num_queues(ctx);
> > + if (n_queues < 0) {
> > + perror("Cannot get number of queues");
> > + return -EFAULT;
> > + }
> > + for (i = 0; i < n_queues / 32; i++)
> > + queue_mask[i] = ~0;
> > + queue_mask[i] = (1 << (n_queues - i * 32)) - 1;
>
> It's unlikely today, I guess, but in theory, this would overflow if n_queues ==
> MAX_NUM_QUEUE
Nice catch, I'll add a check to avoid this.
> > + fprintf(stdout,
> > + "The sub commands will be applied to all %d
> queues\n",
> > + n_queues);
> > + } else {
> > + ctx->argc--;
> > + ctx->argp++;
> > + n_queues = set_queue_mask(queue_mask, *ctx->argp);
> > + if (n_queues < 0) {
> > + perror("Invalid queue mask");
> > + return n_queues;
> > + }
> > + ctx->argc--;
> > + ctx->argp++;
> > + }
> > +
> > + i = find_option(ctx->argc, ctx->argp);
> > + if (i < 0)
> > + exit_bad_args();
> > +
> > + /* no sub_command support yet */
> > +
> > + return 0;
> > +}
>
> Michal Kubecek
^ permalink raw reply
* RE: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue settings
From: Nunley, Nicholas D @ 2019-02-07 23:41 UTC (permalink / raw)
To: Michal Kubecek, netdev@vger.kernel.org
Cc: Kirsher, Jeffrey T, linville@tuxdriver.com, nhorman@redhat.com,
sassmann@redhat.com
In-Reply-To: <20190206103258.GK21401@unicorn.suse.cz>
> -----Original Message-----
> From: Michal Kubecek [mailto:mkubecek@suse.cz]
> Sent: Wednesday, February 6, 2019 2:33 AM
> To: netdev@vger.kernel.org
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; linville@tuxdriver.com;
> Nunley, Nicholas D <nicholas.d.nunley@intel.com>; nhorman@redhat.com;
> sassmann@redhat.com
> Subject: Re: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue
> settings
>
> On Tue, Feb 05, 2019 at 04:01:03PM -0800, Jeff Kirsher wrote:
> > +static int find_max_num_queues(struct cmd_context *ctx) {
> > + struct ethtool_channels echannels;
> > +
> > + echannels.cmd = ETHTOOL_GCHANNELS;
> > + if (send_ioctl(ctx, &echannels))
> > + return -1;
> > +
> > + return MAX(MAX(echannels.rx_count, echannels.tx_count),
> > + echannels.combined_count);
> > +}
>
> Is the outer MAX() correct here? From the documentation to -L option, it
> rather seems we might want
>
> return MAX(echannels.rx_count, echannels.tx_count) +
> echannels.combined_count;
>
> But I can't find any NIC around which would have non-zero rx_count or
> tx_count so that I cannot check.
I think the original assumption here must have been that drivers either use separate Tx/Rx channels or support the combined approach, but never both at the same time. All Intel drivers only support the combined method so I didn't think to second guess this detail of the original implementation, however, I've since looked through the uses of get/set_channels elsewhere in the kernel and it looks like there are a few drivers that support both methods simultaneously, so that clearly needs to be supported too. Your suggestion above looks like the right thing to do.
The device queue-index-to-channel mapping could be a little ambiguous in the mixed mode (and is left as an implementation detail of the individual driver), but I suppose the most sensible approach would be to index through the combined channels first, then move on to the individual Tx/Rx channels, so there shouldn't be any future issues here if these drivers want to add support for per-queue commands.
^ permalink raw reply
* RE: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue settings
From: Nunley, Nicholas D @ 2019-02-07 23:37 UTC (permalink / raw)
To: Michal Kubecek, netdev@vger.kernel.org
Cc: Kirsher, Jeffrey T, linville@tuxdriver.com, nhorman@redhat.com,
sassmann@redhat.com
In-Reply-To: <20190206091832.GI21401@unicorn.suse.cz>
> -----Original Message-----
> From: Michal Kubecek [mailto:mkubecek@suse.cz]
> Sent: Wednesday, February 6, 2019 1:19 AM
> To: netdev@vger.kernel.org
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; linville@tuxdriver.com;
> Nunley, Nicholas D <nicholas.d.nunley@intel.com>; nhorman@redhat.com;
> sassmann@redhat.com
> Subject: Re: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue
> settings
>
> On Tue, Feb 05, 2019 at 04:01:03PM -0800, Jeff Kirsher wrote:
> > Introduce a new ioctl for setting per-queue parameters.
> > Users can apply commands to specific queues by setting SUB_COMMAND
> and
> > queue_mask with the following ethtool command:
> >
> > ethtool --set-perqueue-command DEVNAME [queue_mask %x]
> SUB_COMMAND
>
> The "set" part is IMHO a bit confusing in combination with "show" type
> subcommands.
I'm not a fan of the "set" either. This patch set had already gone through some review before it was passed on to me, and the command naming wasn't a previous point of contention and I didn't want to disturb the parts that weren't "broken", but since you've brought it up I agree that this may not be the best name. I think "--perqueue-command" is more appropriate. Does this seem reasonable to you?
> > +static int set_queue_mask(u32 *queue_mask, char *str) {
> > + int len = strlen(str);
> > + int index = __KERNEL_DIV_ROUND_UP(len * 4, 32);
> > + char tmp[9];
> > + char *end = str + len;
> > + int i, num;
> > + __u32 mask;
> > + int n_queues = 0;
> > +
> > + if (len > MAX_NUM_QUEUE)
> > + return -EINVAL;
> > +
> > + for (i = 0; i < index; i++) {
> > + num = end - str;
> > +
> > + if (num >= 8) {
> > + end -= 8;
> > + num = 8;
> > + } else {
> > + end = str;
> > + }
> > + strncpy(tmp, end, num);
> > + tmp[num] = '\0';
> > +
> > + queue_mask[i] = strtoul(tmp, NULL, 16);
> > +
> > + mask = queue_mask[i];
> > + while (mask > 0) {
> > + if (mask & 0x1)
> > + n_queues++;
> > + mask = mask >> 1;
> > + }
> > + }
> > +
> > + return n_queues;
> > +}
>
> Could you allow optional prefix "0x" as we do for link modes? Maybe
> parse_hex_u32_bitmap() could be used for both.
It's supposed to already work like this through to the eventual call to strtoul() (any "0x" prefix is ignored), but after closer inspection the current implementation won't work for a very specific set of inputs. Depending on the alignment the bitmap substring passed into strtoul() can end up being unrecognizable. For instance, "0xFFFFFFF" ends up getting divided into two calls to strtoul(), "xFFFFFFF" and "0", the former of which ends up evaluating to 0x0 rather than 0xFFFFFFF. Per your suggestion I'll replace the set_queue_mask() functionality with a call to parse_hex_u32_bitmap() to generate the bitmap and avoid this mess, plus some additional code to count up the number of queues.
>
> > +static int do_perqueue(struct cmd_context *ctx) {
> > + __u32
> queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)] = {0};
> > + int i, n_queues = 0;
> > +
> > + if (ctx->argc == 0)
> > + exit_bad_args();
> > +
> > + /*
> > + * The sub commands will be applied to
> > + * all queues if no queue_mask set
> > + */
> > + if (strncmp(*ctx->argp, "queue_mask", 10)) {
> > + n_queues = find_max_num_queues(ctx);
> > + if (n_queues < 0) {
> > + perror("Cannot get number of queues");
> > + return -EFAULT;
> > + }
> > + for (i = 0; i < n_queues / 32; i++)
> > + queue_mask[i] = ~0;
> > + queue_mask[i] = (1 << (n_queues - i * 32)) - 1;
> > + fprintf(stdout,
> > + "The sub commands will be applied to all %d
> queues\n",
> > + n_queues);
> > + } else {
> > + ctx->argc--;
> > + ctx->argp++;
> > + n_queues = set_queue_mask(queue_mask, *ctx->argp);
> > + if (n_queues < 0) {
> > + perror("Invalid queue mask");
> > + return n_queues;
> > + }
> > + ctx->argc--;
> > + ctx->argp++;
> > + }
> > +
> > + i = find_option(ctx->argc, ctx->argp);
> > + if (i < 0)
> > + exit_bad_args();
> > +
> > + /* no sub_command support yet */
> > +
> > + return 0;
> > +}
>
> Technically the return value should be -EOPNOTSUPP here but as the next
> patch fixes that, it doesn't really matter.
I'll fix this anyway since I'll be submitting a new revision.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 5/7] net: phy: marvell10g: Force reading of 2.5/5G PMA extended abilities
From: Russell King - ARM Linux admin @ 2019-02-07 23:37 UTC (permalink / raw)
To: Maxime Chevallier
Cc: Andrew Lunn, davem, netdev, linux-kernel, Florian Fainelli,
Heiner Kallweit, linux-arm-kernel, Antoine Tenart,
thomas.petazzoni, gregory.clement, miquel.raynal, nadavh, stefanc,
mw
In-Reply-To: <20190128152621.2aec96c1@bootlin.com>
On Mon, Jan 28, 2019 at 03:26:21PM +0100, Maxime Chevallier wrote:
> Hello Russell,
>
> On Mon, 21 Jan 2019 13:00:30 +0000
> Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>
> >On Mon, Jan 21, 2019 at 01:29:45PM +0100, Maxime Chevallier wrote:
> >> Hello Russell,
> >>
> >> On Mon, 21 Jan 2019 10:52:06 +0000
> >> Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> >> >It's entirely possible that the 3310 switches to different hardware
> >> >blocks for 2.5G and 5G speeds, and reading _just_ the 1.4 register
> >> >is not sufficient.
> >>
> >> I agree with you but in that particular case, I think we are reading
> >> from the correct device. The datasheet itself says that we should be
> >> reading 1.4 and 1.11 as we expect, with 2.5G/5G support being set (these
> >> registers are read-only, and the datasheet's values aren't what we
> >> actually read).
> >
> >No, you missed what I was saying.
> >
> >The 88x3310 is a hybrid device. It contains multiple instances of
> >each individual device at different offsets in each MMD address space.
>
> Ah I see, I indeed thought you refered to the MMDs.
>
> [...]
>
> >The exception seems to be the PMA/PMD MMD which I've only discovered
> >a single instance.
>
> Yes there only seems to be one. There are some other registers in the
> 1.0xCxxx range, but those who are documented don't help a lot with
> determing wether or not these modes are supported.
>
> I wonder if these values are correctly reported in newer PHY firmware
> revisions.
>
> I've checked other PCS instances, but it seems the one at 3.0x0xxx is
> the one used in 2.5/5GBASET.
In the following, I use "subdevice N" to mean instance "N + 1". So
the instance at address 0 is the main device and the following instance
is subdevice 1.
Looking at the PCS, none of them have the 2.5G/5G bits set in the 3310
which is rather interesting, except the one at 3.0 has the EEE bits
set in 3.21. You are quite correct that the first instance is used
for 2.5G copper, but PCS subdevice 2 also changes as a result of
switching down to 2.5G (its transmit fault status clears.)
It looks like PhyXS subdevice 3 is used (which is a SGMII/1000base-X
MAC facing PHY), is switched to fixed speed mode. It doesn't have
what looks like a sensible advertisement either, so my guess is that
this will need the MAC side to be configured _not_ to expect the
in-band control word in this mode. In other words, it may be SGMII
or 1000base-X upclocked to 2.5G speeds - which it is doesn't matter
because the difference is in the control word which looks like it's
omitted, or if it isn't, doesn't contain useful information.
Note that mvpp2 is slightly buggy in this area, and I have a patch
set that fixes it up - patches can be found in my "mvpp2" branch,
which I'm waiting for my problem reporter to report back after his
testing. If you use this patch set, as long as you don't use
in-band mode, it should be fine.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
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