* Re: [PATCH v2 0/3] Add NETIF_F_HW_BR_CAP feature
From: Vladimir Oltean @ 2019-08-27 14:55 UTC (permalink / raw)
To: Andrew Lunn
Cc: Horatiu Vultur, Roopa Prabhu, nikolay, David S. Miller,
UNGLinuxDriver, Alexandre Belloni, Allan W. Nielsen,
Florian Fainelli, netdev, lkml, bridge
In-Reply-To: <20190827131824.GC11471@lunn.ch>
On Tue, 27 Aug 2019 at 16:20, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > That sounds like a great idea. I was expecting to add this logic in the
> > set_rx_mode function of the driver. But unfortunetly, I got the calls to
> > this function before the dev->promiscuity is updated or not to get the
> > call at all. For example in case the port is member of a bridge and I try
> > to enable the promisc mode.
>
> Hi Horatiu
>
> What about the notifier? Is it called in all the conditions you need
> to know about?
>
> Or, you could consider adding a new switchdev call to pass this
> information to any switchdev driver which is interested in the
> information.
>
> At the moment, the DSA driver core does not pass onto the driver it
> should put a port into promisc mode. So pcap etc, will only see
> traffic directed to the CPU, not all the traffic ingressing the
> interface. If you put the needed core infrastructure into place, we
> could plumb it down from the DSA core to DSA drivers.
>
> Having said that, i don't actually know if the Marvell switches
> support this. Forward using the ATU and send a copy to the CPU? What
> switches tend to support is port mirroring, sending all the traffic
> out another port. A couple of DSA drivers support that, via TC.
>
But the CPU port is not a valid destination for port mirroring in DSA,
I might add.
> Andrew
Regards,
-Vladimir
^ permalink raw reply
* [PATCH V3 net 1/2] openvswitch: Properly set L4 keys on "later" IP fragments
From: Greg Rose @ 2019-08-27 14:58 UTC (permalink / raw)
To: netdev, pshelar; +Cc: joe, Greg Rose
When IP fragments are reassembled before being sent to conntrack, the
key from the last fragment is used. Unless there are reordering
issues, the last fragment received will not contain the L4 ports, so the
key for the reassembled datagram won't contain them. This patch updates
the key once we have a reassembled datagram.
The handle_fragments() function works on L3 headers so we pull the L3/L4
flow key update code from key_extract into a new function
'key_extract_l3l4'. Then we add a another new function
ovs_flow_key_update_l3l4() and export it so that it is accessible by
handle_fragments() for conntrack packet reassembly.
Co-authored by: Justin Pettit <jpettit@ovn.org>
Signed-off-by: Greg Rose <gvrose8192@gmail.com>
---
V1 - V2: Broke out key l3l4 extraction
V2 - V3: Remove code that cleared SW_FLOW_KEY_INVALID for l3l4
extraction
---
net/openvswitch/conntrack.c | 5 ++
net/openvswitch/flow.c | 155 +++++++++++++++++++++++++-------------------
net/openvswitch/flow.h | 1 +
3 files changed, 95 insertions(+), 66 deletions(-)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index d8da647..05249eb 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -525,6 +525,11 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key,
return -EPFNOSUPPORT;
}
+ /* The key extracted from the fragment that completed this datagram
+ * likely didn't have an L4 header, so regenerate it.
+ */
+ ovs_flow_key_update_l3l4(skb, key);
+
key->ip.frag = OVS_FRAG_TYPE_NONE;
skb_clear_hash(skb);
skb->ignore_df = 1;
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index bc89e16..005f762 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -523,78 +523,15 @@ static int parse_nsh(struct sk_buff *skb, struct sw_flow_key *key)
}
/**
- * key_extract - extracts a flow key from an Ethernet frame.
+ * key_extract_l3l4 - extracts L3/L4 header information.
* @skb: sk_buff that contains the frame, with skb->data pointing to the
- * Ethernet header
+ * L3 header
* @key: output flow key
*
- * The caller must ensure that skb->len >= ETH_HLEN.
- *
- * Returns 0 if successful, otherwise a negative errno value.
- *
- * Initializes @skb header fields as follows:
- *
- * - skb->mac_header: the L2 header.
- *
- * - skb->network_header: just past the L2 header, or just past the
- * VLAN header, to the first byte of the L2 payload.
- *
- * - skb->transport_header: If key->eth.type is ETH_P_IP or ETH_P_IPV6
- * on output, then just past the IP header, if one is present and
- * of a correct length, otherwise the same as skb->network_header.
- * For other key->eth.type values it is left untouched.
- *
- * - skb->protocol: the type of the data starting at skb->network_header.
- * Equals to key->eth.type.
*/
-static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
+static int key_extract_l3l4(struct sk_buff *skb, struct sw_flow_key *key)
{
int error;
- struct ethhdr *eth;
-
- /* Flags are always used as part of stats */
- key->tp.flags = 0;
-
- skb_reset_mac_header(skb);
-
- /* Link layer. */
- clear_vlan(key);
- if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
- if (unlikely(eth_type_vlan(skb->protocol)))
- return -EINVAL;
-
- skb_reset_network_header(skb);
- key->eth.type = skb->protocol;
- } else {
- eth = eth_hdr(skb);
- ether_addr_copy(key->eth.src, eth->h_source);
- ether_addr_copy(key->eth.dst, eth->h_dest);
-
- __skb_pull(skb, 2 * ETH_ALEN);
- /* We are going to push all headers that we pull, so no need to
- * update skb->csum here.
- */
-
- if (unlikely(parse_vlan(skb, key)))
- return -ENOMEM;
-
- key->eth.type = parse_ethertype(skb);
- if (unlikely(key->eth.type == htons(0)))
- return -ENOMEM;
-
- /* Multiple tagged packets need to retain TPID to satisfy
- * skb_vlan_pop(), which will later shift the ethertype into
- * skb->protocol.
- */
- if (key->eth.cvlan.tci & htons(VLAN_CFI_MASK))
- skb->protocol = key->eth.cvlan.tpid;
- else
- skb->protocol = key->eth.type;
-
- skb_reset_network_header(skb);
- __skb_push(skb, skb->data - skb_mac_header(skb));
- }
- skb_reset_mac_len(skb);
/* Network layer. */
if (key->eth.type == htons(ETH_P_IP)) {
@@ -788,6 +725,92 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
return 0;
}
+/**
+ * key_extract - extracts a flow key from an Ethernet frame.
+ * @skb: sk_buff that contains the frame, with skb->data pointing to the
+ * Ethernet header
+ * @key: output flow key
+ *
+ * The caller must ensure that skb->len >= ETH_HLEN.
+ *
+ * Returns 0 if successful, otherwise a negative errno value.
+ *
+ * Initializes @skb header fields as follows:
+ *
+ * - skb->mac_header: the L2 header.
+ *
+ * - skb->network_header: just past the L2 header, or just past the
+ * VLAN header, to the first byte of the L2 payload.
+ *
+ * - skb->transport_header: If key->eth.type is ETH_P_IP or ETH_P_IPV6
+ * on output, then just past the IP header, if one is present and
+ * of a correct length, otherwise the same as skb->network_header.
+ * For other key->eth.type values it is left untouched.
+ *
+ * - skb->protocol: the type of the data starting at skb->network_header.
+ * Equals to key->eth.type.
+ */
+static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ struct ethhdr *eth;
+
+ /* Flags are always used as part of stats */
+ key->tp.flags = 0;
+
+ skb_reset_mac_header(skb);
+
+ /* Link layer. */
+ clear_vlan(key);
+ if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
+ if (unlikely(eth_type_vlan(skb->protocol)))
+ return -EINVAL;
+
+ skb_reset_network_header(skb);
+ key->eth.type = skb->protocol;
+ } else {
+ eth = eth_hdr(skb);
+ ether_addr_copy(key->eth.src, eth->h_source);
+ ether_addr_copy(key->eth.dst, eth->h_dest);
+
+ __skb_pull(skb, 2 * ETH_ALEN);
+ /* We are going to push all headers that we pull, so no need to
+ * update skb->csum here.
+ */
+
+ if (unlikely(parse_vlan(skb, key)))
+ return -ENOMEM;
+
+ key->eth.type = parse_ethertype(skb);
+ if (unlikely(key->eth.type == htons(0)))
+ return -ENOMEM;
+
+ /* Multiple tagged packets need to retain TPID to satisfy
+ * skb_vlan_pop(), which will later shift the ethertype into
+ * skb->protocol.
+ */
+ if (key->eth.cvlan.tci & htons(VLAN_CFI_MASK))
+ skb->protocol = key->eth.cvlan.tpid;
+ else
+ skb->protocol = key->eth.type;
+
+ skb_reset_network_header(skb);
+ __skb_push(skb, skb->data - skb_mac_header(skb));
+ }
+
+ skb_reset_mac_len(skb);
+
+ /* Fill out L3/L4 key info, if any */
+ return key_extract_l3l4(skb, key);
+}
+
+/* In the case of conntrack fragment handling it expects L3 headers,
+ * add a helper.
+ */
+int ovs_flow_key_update_l3l4(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ return key_extract_l3l4(skb, key);
+}
+
int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
{
int res;
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index a5506e2..b830d5f 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -270,6 +270,7 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
u64 ovs_flow_used_time(unsigned long flow_jiffies);
int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key);
+int ovs_flow_key_update_l3l4(struct sk_buff *skb, struct sw_flow_key *key);
int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
struct sk_buff *skb,
struct sw_flow_key *key);
--
1.8.3.1
^ permalink raw reply related
* [PATCH V3 net 2/2] openvswitch: Clear the L4 portion of the key for "later" fragments.
From: Greg Rose @ 2019-08-27 14:58 UTC (permalink / raw)
To: netdev, pshelar; +Cc: joe, Justin Pettit
In-Reply-To: <1566917890-22304-1-git-send-email-gvrose8192@gmail.com>
From: Justin Pettit <jpettit@ovn.org>
Only the first fragment in a datagram contains the L4 headers. When the
Open vSwitch module parses a packet, it always sets the IP protocol
field in the key, but can only set the L4 fields on the first fragment.
The original behavior would not clear the L4 portion of the key, so
garbage values would be sent in the key for "later" fragments. This
patch clears the L4 fields in that circumstance to prevent sending those
garbage values as part of the upcall.
Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
net/openvswitch/flow.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 005f762..9d81d2c 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -560,6 +560,7 @@ static int key_extract_l3l4(struct sk_buff *skb, struct sw_flow_key *key)
offset = nh->frag_off & htons(IP_OFFSET);
if (offset) {
key->ip.frag = OVS_FRAG_TYPE_LATER;
+ memset(&key->tp, 0, sizeof(key->tp));
return 0;
}
if (nh->frag_off & htons(IP_MF) ||
@@ -677,8 +678,10 @@ static int key_extract_l3l4(struct sk_buff *skb, struct sw_flow_key *key)
return error;
}
- if (key->ip.frag == OVS_FRAG_TYPE_LATER)
+ if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
+ memset(&key->tp, 0, sizeof(key->tp));
return 0;
+ }
if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
key->ip.frag = OVS_FRAG_TYPE_FIRST;
--
1.8.3.1
^ permalink raw reply related
* Re: kernel/bpf/core.o: warning: objtool: ___bpf_prog_run.cold()+0x7: call without frame pointer save/setup
From: Josh Poimboeuf @ 2019-08-27 14:58 UTC (permalink / raw)
To: He Zhe
Cc: ast, daniel, kafai, songliubraving, yhs, ndesaulniers,
miguel.ojeda.sandonis, luc.vanoostenryck, schwidefsky, gregkh,
mst, gor, andreyknvl, liuxiaozhou, yamada.masahiro, linux-kernel,
netdev, bpf
In-Reply-To: <2c416fe7-f6be-440b-b476-9fede1ea123c@windriver.com>
On Tue, Aug 27, 2019 at 10:43:27AM +0800, He Zhe wrote:
>
>
> On 8/26/19 11:18 PM, Josh Poimboeuf wrote:
> > On Mon, Aug 26, 2019 at 10:42:53PM +0800, He Zhe wrote:
> >> Hi All,
> >>
> >> Since 3193c0836f20 ("bpf: Disable GCC -fgcse optimization for ___bpf_prog_run()"),
> >> We have got the following warning,
> >> kernel/bpf/core.o: warning: objtool: ___bpf_prog_run.cold()+0x7: call without frame pointer save/setup
> >>
> >> If reverting the above commit, we will get the following warning,
> >> kernel/bpf/core.o: warning: objtool: ___bpf_prog_run()+0x8b9: sibling call from callable instruction with modified stack frame
> >> if CONFIG_RETPOLINE=n, and no warning if CONFIG_RETPOLINE=y
> > Can you please share the following:
> >
> > - core.o file
>
> Attached.
>
> >
> > The following would also be helpful for me to try to recreate it:
> >
> > - config file
> > - compiler version
> > - kernel version
>
> I pasted them in the other reply.
Thanks. I was able to recreate. I reduced it to:
void a(b);
__attribute__((optimize(""))) c(void) { a(); }
Apparently '__attribute__((optimize()))' is overwriting GCC cmdline
flags, including -fno-omit-frame-pointer. I had assumed it would append
instead of replace.
I'm guessing this is a GCC "feature" instead of a bug. I'll need to
follow up.
--
Josh
^ permalink raw reply
* Re: [PATCH v1 net-next 4/4] net: stmmac: setup higher frequency clk support for EHL & TGL
From: Florian Fainelli @ 2019-08-27 15:08 UTC (permalink / raw)
To: Voon, Weifeng, Andrew Lunn
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Giuseppe Cavallaro,
Alexandre Torgue, Ong, Boon Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC814758DB5@PGSMSX103.gar.corp.intel.com>
On 8/27/2019 3:38 AM, Voon, Weifeng wrote:
>>>> +#include <linux/clk-provider.h>
>>>> #include <linux/pci.h>
>>>> #include <linux/dmi.h>
>>>>
>>>> @@ -174,6 +175,19 @@ static int intel_mgbe_common_data(struct
>> pci_dev *pdev,
>>>> plat->axi->axi_blen[1] = 8;
>>>> plat->axi->axi_blen[2] = 16;
>>>>
>>>> + plat->ptp_max_adj = plat->clk_ptp_rate;
>>>> +
>>>> + /* Set system clock */
>>>> + plat->stmmac_clk = clk_register_fixed_rate(&pdev->dev,
>>>> + "stmmac-clk", NULL, 0,
>>>> + plat->clk_ptp_rate);
>>>> +
>>>> + if (IS_ERR(plat->stmmac_clk)) {
>>>> + dev_warn(&pdev->dev, "Fail to register stmmac-clk\n");
>>>> + plat->stmmac_clk = NULL;
>>>
>>> Don't you need to propagate at least EPROBE_DEFER here?
>>
>> Hi Florian
>>
>> Isn't a fixed rate clock a complete fake. There is no hardware behind it.
>> So can it return EPROBE_DEFER?
>>
>> Andrew
>
> Yes, there is no hardware behind it. So, I don't think we need to deferred probe
> and a warning message should be sufficient. Anyhow, please point it out if I miss
> out anything.
Looks good to me, thanks both for clarifying.
--
Florian
^ permalink raw reply
* Re: BUG_ON in skb_segment, after bpf_skb_change_proto was applied
From: Eric Dumazet @ 2019-08-27 15:09 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: Daniel Borkmann, netdev, Alexander Duyck, Alexei Starovoitov,
Yonghong Song, Steffen Klassert, shmulik, eyal
In-Reply-To: <20190827144218.5b098eac@pixies>
On 8/27/19 1:42 PM, Shmulik Ladkani wrote:
> On Mon, 26 Aug 2019 19:47:40 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> On 8/26/19 4:07 PM, Shmulik Ladkani wrote:
>>> - ipv4 forwarding to dummy1, where eBPF nat4-to-6 program is attached
>>> at TC Egress (calls 'bpf_skb_change_proto()'), then redirect to ingress
>>> on same device.
>>> NOTE: 'bpf_skb_proto_4_to_6()' mangles 'shinfo->gso_size'
>>
>> Doing this on an skb with a frag_list is doomed, in current gso_segment() state.
>>
>> A rewrite would be needed (I believe I did so at some point, but Herbert Xu fought hard against it)
>
> Thanks Eric,
>
> - If a rewrite is still considered out of the question, how can one use
> eBPF's bpf_skb_change_proto() safely without disabling GRO completely?
> - e.g. is there a way to force the GROed skbs to fit into a layout that is
> tolerated by skb_segment?
> - alternatively can eBPF layer somehow enforce segmentation of the
> original GROed skb before mangling the gso_size?
>
> - Another thing that puzzles me is that we hit the BUG_ON rather rarely
> and cannot yet reproduce synthetically. If skb_segment's handling of
> skbs with a frag_list (that have gso_size mangled) is broken, I'd expect
> to hit this more often... Any ideas?
skb_segment of a gro packet (especially with frag_list) is only supported
if the geometry of the individual segments is not changed,
meaning that gso_size must remain the same.
>
> - Suppose going for a rewrite, care to elaborate what's exactly missing
> in skb_segment's logic?
> I must admit I do not fully understand all the different code flows in
> this function, it seems to support many different input skbs - any
> assistance is highly appreciated.
Well, this is the point really.
The complexity of this function is so high that very few of us dare to touch it.
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Roopa Prabhu @ 2019-08-27 15:14 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Jakub Kicinski, David Ahern, netdev,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
In-Reply-To: <20190827093525.GB2250@nanopsycho>
On Tue, Aug 27, 2019 at 2:35 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, Aug 27, 2019 at 10:22:42AM CEST, davem@davemloft.net wrote:
> >From: Jiri Pirko <jiri@resnulli.us>
> >Date: Tue, 27 Aug 2019 09:08:08 +0200
> >
> >> Okay, so if I understand correctly, on top of separate commands for
> >> add/del of alternative names, you suggest also get/dump to be separate
> >> command and don't fill this up in existing newling/getlink command.
> >
> >I'm not sure what to do yet.
> >
> >David has a point, because the only way these ifnames are useful is
> >as ways to specify and choose net devices. So based upon that I'm
> >slightly learning towards not using separate commands.
>
> Well yeah, one can use it to handle existing commands instead of
> IFLA_NAME.
>
> But why does it rule out separate commands? I think it is cleaner than
> to put everything in poor setlink messages :/ The fact that we would
> need to add "OP" to the setlink message just feels of. Other similar
> needs may show up in the future and we may endup in ridiculous messages
> like:
>
> SETLINK
> IFLA_NAME eth0
> IFLA_ATLNAME_LIST (nest)
> IFLA_ALTNAME_OP add
> IFLA_ALTNAME somereallylongname
> IFLA_ALTNAME_OP del
> IFLA_ALTNAME somereallyreallylongname
> IFLA_ALTNAME_OP add
> IFLA_ALTNAME someotherreallylongname
> IFLA_SOMETHING_ELSE_LIST (nest)
> IFLA_SOMETHING_ELSE_OP add
> ...
> IFLA_SOMETHING_ELSE_OP del
> ...
> IFLA_SOMETHING_ELSE_OP add
> ...
>
> I don't know what to think about it. Rollbacks are going to be pure hell :/
I don't see a huge problem with the above. We need a way to solve this
anyways for other list types in the future correct ?.
The approach taken by this series will not scale if we have to add a
new msg type and header for every such list attribute in the future.
A good parallel here is bridge vlan which uses RTM_SETLINK and
RTM_DELLINK for vlan add and deletes. But it does have an advantage of
a separate
msg space under AF_BRIDGE which makes it cleaner. Maybe something
closer to that can be made to work (possibly with a msg flag) ?.
Would be good to have a consistent way to update list attributes for
future needs too.
^ permalink raw reply
* Re: [PATCH] ipv6: Not to probe neighbourless routes
From: Eric Dumazet @ 2019-08-27 15:22 UTC (permalink / raw)
To: Yi Wang, davem
Cc: kuznet, yoshfuji, netdev, linux-kernel, xue.zhihong, wang.liang82,
Cheng Lin
In-Reply-To: <1566896907-5121-1-git-send-email-wang.yi59@zte.com.cn>
On 8/27/19 11:08 AM, Yi Wang wrote:
> From: Cheng Lin <cheng.lin130@zte.com.cn>
>
> Originally, Router Reachability Probing require a neighbour entry
> existed. Commit 2152caea7196 ("ipv6: Do not depend on rt->n in
> rt6_probe().") removed the requirement for a neighbour entry. And
> commit f547fac624be ("ipv6: rate-limit probes for neighbourless
> routes") adds rate-limiting for neighbourless routes.
>
> And, the Neighbor Discovery for IP version 6 (IPv6)(rfc4861) says,
> "
> 7.2.5. Receipt of Neighbor Advertisements
>
> When a valid Neighbor Advertisement is received (either solicited or
> unsolicited), the Neighbor Cache is searched for the target's entry.
> If no entry exists, the advertisement SHOULD be silently discarded.
> There is no need to create an entry if none exists, since the
> recipient has apparently not initiated any communication with the
> target.
> ".
>
> In rt6_probe(), just a Neighbor Solicitation message are transmited.
> When receiving a Neighbor Advertisement, the node does nothing in a
> Neighborless condition.
>
> Not sure it's needed to create a neighbor entry in Router
> Reachability Probing. And the Original way may be the right way.
>
> This patch recover the requirement for a neighbour entry.
>
> Signed-off-by: Cheng Lin <cheng.lin130@zte.com.cn>
> ---
> include/net/ip6_fib.h | 5 -----
> net/ipv6/route.c | 5 +----
> 2 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index 4b5656c..8c2e022 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -124,11 +124,6 @@ struct rt6_exception {
>
> struct fib6_nh {
> struct fib_nh_common nh_common;
> -
> -#ifdef CONFIG_IPV6_ROUTER_PREF
> - unsigned long last_probe;
> -#endif
> -
> struct rt6_info * __percpu *rt6i_pcpu;
> struct rt6_exception_bucket __rcu *rt6i_exception_bucket;
> };
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index fd059e0..c4bcffc 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -639,12 +639,12 @@ static void rt6_probe(struct fib6_nh *fib6_nh)
> nh_gw = &fib6_nh->fib_nh_gw6;
> dev = fib6_nh->fib_nh_dev;
> rcu_read_lock_bh();
> - idev = __in6_dev_get(dev);
> neigh = __ipv6_neigh_lookup_noref(dev, nh_gw);
> if (neigh) {
> if (neigh->nud_state & NUD_VALID)
> goto out;
>
> + idev = __in6_dev_get(dev);
> write_lock(&neigh->lock);
> if (!(neigh->nud_state & NUD_VALID) &&
> time_after(jiffies,
> @@ -654,9 +654,6 @@ static void rt6_probe(struct fib6_nh *fib6_nh)
> __neigh_set_probe_once(neigh);
> }
> write_unlock(&neigh->lock);
> - } else if (time_after(jiffies, fib6_nh->last_probe +
> - idev->cnf.rtr_probe_interval)) {
> - work = kmalloc(sizeof(*work), GFP_ATOMIC);
> }
>
> if (work) {
>
Have you really compiled this patch ?
^ permalink raw reply
* RE: [PATCH v1 net-next] net: phy: mdio_bus: make mdiobus_scan also cover PHY that only talks C45
From: Voon, Weifeng @ 2019-08-27 15:23 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Heiner Kallweit,
Ong, Boon Leong
In-Reply-To: <20190826185418.GG2168@lunn.ch>
> > > Make mdiobus_scan() to try harder to look for any PHY that only
> talks C45.
> > If you are not using Device Tree or ACPI, and you are letting the MDIO
> > bus be scanned, it sounds like there should be a way for you to
> > provide a hint as to which addresses should be scanned (that's
> > mii_bus::phy_mask) and possibly enhance that with a mask of possible
> > C45 devices?
>
> Yes, i don't like this unconditional c45 scanning. A lot of MDIO bus
> drivers don't look for the MII_ADDR_C45. They are going to do a C22
> transfer, and maybe not mask out the MII_ADDR_C45 from reg, causing an
> invalid register write. Bad things can then happen.
>
> With DT and ACPI, we have an explicit indication that C45 should be used,
> so we know on this platform C45 is safe to use. We need something
> similar when not using DT or ACPI.
>
> Andrew
Florian and Andrew,
The mdio c22 is using the start-of-frame ST=01 while mdio c45 is using ST=00
as identifier. So mdio c22 device will not response to mdio c45 protocol.
As in IEEE 802.1ae-2002 Annex 45A.3 mention that:
" Even though the Clause 45 MDIO frames using the ST=00 frame code
will also be driven on to the Clause 22 MII Management interface,
the Clause 22 PHYs will ignore the frames. "
Hence, I am not seeing any concern that the c45 scanning will mess up with
c22 devices.
Weifeng
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Alex Williamson @ 2019-08-27 15:23 UTC (permalink / raw)
To: Parav Pandit
Cc: Mark Bloch, Jiri Pirko, kwankhede@nvidia.com, cohuck@redhat.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866BB4736D265EF28280014D1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 04:28:37 +0000
Parav Pandit <parav@mellanox.com> wrote:
> Hi Mark,
>
> > -----Original Message-----
> > From: Mark Bloch <markb@mellanox.com>
> > Sent: Tuesday, August 27, 2019 4:32 AM
> > To: Parav Pandit <parav@mellanox.com>; alex.williamson@redhat.com; Jiri
> > Pirko <jiri@mellanox.com>; kwankhede@nvidia.com; cohuck@redhat.com;
> > davem@davemloft.net
> > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org
> > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> >
> >
> >
> > On 8/26/19 1:41 PM, Parav Pandit wrote:
> > > Mdev alias should be unique among all the mdevs, so that when such
> > > alias is used by the mdev users to derive other objects, there is no
> > > collision in a given system.
> > >
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > ---
> > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > 100644
> > > --- a/drivers/vfio/mdev/mdev_core.c
> > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> > struct device *dev,
> > > ret = -EEXIST;
> > > goto mdev_fail;
> > > }
> > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> >
> > alias can be NULL here no?
> >
> If alias is NULL, tmp->alias would also be null because for given parent either we have alias or we don’t.
> So its not possible to have tmp->alias as null and alias as non null.
> But it may be good/defensive to add check for both.
mdev_list is a global list of all mdev devices, how can we make any
assumptions that an element has the same parent? Thanks,
Alex
> > > + mutex_unlock(&mdev_list_lock);
> > > + ret = -EEXIST;
> > > + goto mdev_fail;
> > > + }
> > > }
> > >
> > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > >
> >
> > Mark
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Alex Williamson @ 2019-08-27 15:28 UTC (permalink / raw)
To: Cornelia Huck
Cc: Parav Pandit, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827132946.0b92d259.cohuck@redhat.com>
On Tue, 27 Aug 2019 13:29:46 +0200
Cornelia Huck <cohuck@redhat.com> wrote:
> On Tue, 27 Aug 2019 11:08:59 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> > >
> > > On Mon, 26 Aug 2019 15:41:17 -0500
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > Mdev alias should be unique among all the mdevs, so that when such
> > > > alias is used by the mdev users to derive other objects, there is no
> > > > collision in a given system.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > ---
> > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > 100644
> > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj, struct
> > > device *dev,
> > > > ret = -EEXIST;
> > > > goto mdev_fail;
> > > > }
> > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > >
> > > Any way we can relay to the caller that the uuid was fine, but that we had a
> > > hash collision? Duplicate uuids are much more obvious than a collision here.
> > >
> > How do you want to relay this rare event?
> > Netlink interface has way to return the error message back, but sysfs is limited due to its error code based interface.
>
> I don't know, that's why I asked :)
>
> The problem is that "uuid already used" and "hash collision" are
> indistinguishable. While "use a different uuid" will probably work in
> both cases, "increase alias length" might be a good alternative in some
> cases.
>
> But if there is no good way to relay the problem, we can live with it.
It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\" for mdev device %pUl\n",...
Thanks,
Alex
> > > > + mutex_unlock(&mdev_list_lock);
> > > > + ret = -EEXIST;
> > > > + goto mdev_fail;
> > > > + }
> > > > }
> > > >
> > > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> >
>
^ permalink raw reply
* Re: [net-next] net: sched: pie: enable timestamp based delay calculation
From: Eric Dumazet @ 2019-08-27 15:34 UTC (permalink / raw)
To: Gautam Ramakrishnan, netdev
Cc: jhs, davem, xiyou.wangcong, Leslie Monis, Mohit P . Tahiliani,
Dave Taht
In-Reply-To: <20190827141938.23483-1-gautamramk@gmail.com>
On 8/27/19 4:19 PM, Gautam Ramakrishnan wrote:
> RFC 8033 suggests an alternative approach to calculate the queue
> delay in PIE by using per packet timestamps. This patch enables the
> PIE implementation to do this.
>
> The calculation of queue delay is as follows:
> qdelay = now - packet_enqueue_time
>
> To enable the use of timestamps:
> modprobe sch_pie use_timestamps=1
No module parameter is accepted these days.
Please add a new attribute instead,
so that pie can be used in both mode on the same host.
For a typical example of attribute addition, please take
a look at commit 48872c11b77271ef9b070bdc50afe6655c4eb9aa
("net_sched: sch_fq: add dctcp-like marking")
^ permalink raw reply
* Re: [PATCH v1 net-next] net: phy: mdio_bus: make mdiobus_scan also cover PHY that only talks C45
From: Florian Fainelli @ 2019-08-27 15:37 UTC (permalink / raw)
To: Voon, Weifeng, Andrew Lunn
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Heiner Kallweit,
Ong, Boon Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC814758ED8@PGSMSX103.gar.corp.intel.com>
On 8/27/2019 8:23 AM, Voon, Weifeng wrote:
>>>> Make mdiobus_scan() to try harder to look for any PHY that only
>> talks C45.
>>> If you are not using Device Tree or ACPI, and you are letting the MDIO
>>> bus be scanned, it sounds like there should be a way for you to
>>> provide a hint as to which addresses should be scanned (that's
>>> mii_bus::phy_mask) and possibly enhance that with a mask of possible
>>> C45 devices?
>>
>> Yes, i don't like this unconditional c45 scanning. A lot of MDIO bus
>> drivers don't look for the MII_ADDR_C45. They are going to do a C22
>> transfer, and maybe not mask out the MII_ADDR_C45 from reg, causing an
>> invalid register write. Bad things can then happen.
>>
>> With DT and ACPI, we have an explicit indication that C45 should be used,
>> so we know on this platform C45 is safe to use. We need something
>> similar when not using DT or ACPI.
>>
>> Andrew
>
> Florian and Andrew,
> The mdio c22 is using the start-of-frame ST=01 while mdio c45 is using ST=00
> as identifier. So mdio c22 device will not response to mdio c45 protocol.
> As in IEEE 802.1ae-2002 Annex 45A.3 mention that:
> " Even though the Clause 45 MDIO frames using the ST=00 frame code
> will also be driven on to the Clause 22 MII Management interface,
> the Clause 22 PHYs will ignore the frames. "
>
> Hence, I am not seeing any concern that the c45 scanning will mess up with
> c22 devices.
It is not so much the messing up that concerns me other than the
increased scan time. Assuming you are making this change to support your
stmmac PCI patch series with SGMII/RGMII, etc. cannot you introduce a
bitmask of C45 PHY addresses that should be scanned and the logic could
look like (pseudo code):
- for each bit clear in mii_bus::phy_mask, scan it as C22
- for each bit clear in mii_bus::phy_c45_mask, scan it as C45
or something along those lines?
--
Florian
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Cornelia Huck @ 2019-08-27 15:39 UTC (permalink / raw)
To: Alex Williamson
Cc: Parav Pandit, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827092855.29702347@x1.home>
On Tue, 27 Aug 2019 09:28:55 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Tue, 27 Aug 2019 13:29:46 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Tue, 27 Aug 2019 11:08:59 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> > > >
> > > > On Mon, 26 Aug 2019 15:41:17 -0500
> > > > Parav Pandit <parav@mellanox.com> wrote:
> > > >
> > > > > Mdev alias should be unique among all the mdevs, so that when such
> > > > > alias is used by the mdev users to derive other objects, there is no
> > > > > collision in a given system.
> > > > >
> > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > ---
> > > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > > 100644
> > > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj, struct
> > > > device *dev,
> > > > > ret = -EEXIST;
> > > > > goto mdev_fail;
> > > > > }
> > > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > > >
> > > > Any way we can relay to the caller that the uuid was fine, but that we had a
> > > > hash collision? Duplicate uuids are much more obvious than a collision here.
> > > >
> > > How do you want to relay this rare event?
> > > Netlink interface has way to return the error message back, but sysfs is limited due to its error code based interface.
> >
> > I don't know, that's why I asked :)
> >
> > The problem is that "uuid already used" and "hash collision" are
> > indistinguishable. While "use a different uuid" will probably work in
> > both cases, "increase alias length" might be a good alternative in some
> > cases.
> >
> > But if there is no good way to relay the problem, we can live with it.
>
> It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\" for mdev device %pUl\n",...
Sounds good to me.
^ permalink raw reply
* Re: [PATCH rdma-next v3 0/3] ODP support for mlx5 DC QPs
From: Jason Gunthorpe @ 2019-08-27 15:51 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list,
Michael Guralnik, Saeed Mahameed, linux-netdev
In-Reply-To: <20190819120815.21225-1-leon@kernel.org>
On Mon, Aug 19, 2019 at 03:08:12PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog
> v3:
> * Rewrote patches to expose through DEVX without need to change mlx5-abi.h at all.
> v2: https://lore.kernel.org/linux-rdma/20190806074807.9111-1-leon@kernel.org
> * Fixed reserved_* field wrong name (Saeed M.)
> * Split first patch to two patches, one for mlx5-next and one for rdma-next. (Saeed M.)
> v1: https://lore.kernel.org/linux-rdma/20190804100048.32671-1-leon@kernel.org
> * Fixed alignment to u64 in mlx5-abi.h (Gal P.)
> v0: https://lore.kernel.org/linux-rdma/20190801122139.25224-1-leon@kernel.org
>
> >From Michael,
>
> The series adds support for on-demand paging for DC transport.
>
> As DC is mlx-only transport, the capabilities are exposed
> to the user using DEVX objects and later on through mlx5dv_query_device.
>
> Thanks
>
> Michael Guralnik (3):
> net/mlx5: Set ODP capabilities for DC transport to max
> IB/mlx5: Remove check of FW capabilities in ODP page fault handling
> IB/mlx5: Add page fault handler for DC initiator WQE
This seems fine, can you put the commit on the shared branch?
Thanks,
Jason
^ permalink raw reply
* [PATCH net] ibmvnic: Do not process reset during or after device removal
From: Thomas Falcon @ 2019-08-27 16:10 UTC (permalink / raw)
To: netdev; +Cc: linuxppc-dev, Thomas Falcon
Currently, the ibmvnic driver will not schedule device resets
if the device is being removed, but does not check the device
state before the reset is actually processed. This leads to a race
where a reset is scheduled with a valid device state but is
processed after the driver has been removed, resulting in an oops.
Fix this by checking the device state before processing a queued
reset event.
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Tested-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index cebd20f..fa4bb94 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1983,6 +1983,10 @@ static void __ibmvnic_reset(struct work_struct *work)
rwi = get_next_rwi(adapter);
while (rwi) {
+ if (adapter->state == VNIC_REMOVING ||
+ adapter->state == VNIC_REMOVED)
+ goto out;
+
if (adapter->force_reset_recovery) {
adapter->force_reset_recovery = false;
rc = do_hard_reset(adapter, rwi, reset_state);
@@ -2007,7 +2011,7 @@ static void __ibmvnic_reset(struct work_struct *work)
netdev_dbg(adapter->netdev, "Reset failed\n");
free_all_rwi(adapter);
}
-
+out:
adapter->resetting = false;
if (we_lock_rtnl)
rtnl_unlock();
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v6 1/4] dt-bindings: net: phy: Add subnode for LED configuration
From: Rob Herring @ 2019-08-27 16:12 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Mark Rutland, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190813191147.19936-2-mka@chromium.org>
On Tue, Aug 13, 2019 at 12:11:44PM -0700, Matthias Kaehlcke wrote:
> The LED behavior of some Ethernet PHYs is configurable. Add an
> optional 'leds' subnode with a child node for each LED to be
> configured. The binding aims to be compatible with the common
> LED binding (see devicetree/bindings/leds/common.txt).
>
> A LED can be configured to be:
>
> - 'on' when a link is active, some PHYs allow configuration for
> certain link speeds
> speeds
> - 'off'
> - blink on RX/TX activity, some PHYs allow configuration for
> certain link speeds
>
> For the configuration to be effective it needs to be supported by
> the hardware and the corresponding PHY driver.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v6:
> - none
>
> Changes in v5:
> - renamed triggers from 'phy_link_<speed>_active' to 'phy-link-<speed>'
> - added entries for 'phy-link-<speed>-activity'
> - added 'phy-link' and 'phy-link-activity' for triggers with any link
> speed
> - added entry for trigger 'none'
>
> Changes in v4:
> - patch added to the series
> ---
> .../devicetree/bindings/net/ethernet-phy.yaml | 59 +++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index f70f18ff821f..98ba320f828b 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -153,6 +153,50 @@ properties:
> Delay after the reset was deasserted in microseconds. If
> this property is missing the delay will be skipped.
>
> +patternProperties:
> + "^leds$":
> + type: object
> + description:
> + Subnode with configuration of the PHY LEDs.
#address-cells and #size-cells needed.
> +
> + patternProperties:
> + "^led@[0-9]+$":
Need to allow for the case of 1 LED which doesn't need a unit-address
nor reg.
> + type: object
> + description:
> + Subnode with the configuration of a single PHY LED.
> +
> + properties:
> + reg:
> + description:
> + The ID number of the LED, typically corresponds to a hardware ID.
> + $ref: "/schemas/types.yaml#/definitions/uint32"
Standard properties already have a type definition. What's needed is
'maxItems: 1'.
> +
> + linux,default-trigger:
> + description:
> + This parameter, if present, is a string specifying the trigger
> + assigned to the LED. Supported triggers are:
> + "none" - LED will be solid off
> + "phy-link" - LED will be solid on when a link is active
> + "phy-link-10m" - LED will be solid on when a 10Mb/s link is active
> + "phy-link-100m" - LED will be solid on when a 100Mb/s link is active
> + "phy-link-1g" - LED will be solid on when a 1Gb/s link is active
> + "phy-link-10g" - LED will be solid on when a 10Gb/s link is active
> + "phy-link-activity" - LED will be on when link is active and blink
> + off with activity.
> + "phy-link-10m-activity" - LED will be on when 10Mb/s link is active
> + and blink off with activity.
> + "phy-link-100m-activity" - LED will be on when 100Mb/s link is
> + active and blink off with activity.
> + "phy-link-1g-activity" - LED will be on when 1Gb/s link is active
> + and blink off with activity.
> + "phy-link-10g-activity" - LED will be on when 10Gb/s link is active
> + and blink off with activity.
These strings all need to be in an enum.
The led binding is moving away from linux,default-trigger to 'function'
and 'color' properties. You probably want to do that here.
> +
> + $ref: "/schemas/types.yaml#/definitions/string"
> +
> + required:
> + - reg
> +
> required:
> - reg
>
> @@ -173,5 +217,20 @@ examples:
> reset-gpios = <&gpio1 4 1>;
> reset-assert-us = <1000>;
> reset-deassert-us = <2000>;
> +
> + leds {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@0 {
> + reg = <0>;
> + linux,default-trigger = "phy-link-1g";
> + };
> +
> + led@1 {
> + reg = <1>;
> + linux,default-trigger = "phy-link-100m-activity";
> + };
> + };
> };
> };
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [PATCH v1 net-next] net: phy: mdio_bus: make mdiobus_scan also cover PHY that only talks C45
From: Andrew Lunn @ 2019-08-27 15:49 UTC (permalink / raw)
To: Voon, Weifeng
Cc: Florian Fainelli, David S. Miller, Maxime Coquelin,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Jose Abreu,
Heiner Kallweit, Ong, Boon Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC814758ED8@PGSMSX103.gar.corp.intel.com>
On Tue, Aug 27, 2019 at 03:23:34PM +0000, Voon, Weifeng wrote:
> > > > Make mdiobus_scan() to try harder to look for any PHY that only
> > talks C45.
> > > If you are not using Device Tree or ACPI, and you are letting the MDIO
> > > bus be scanned, it sounds like there should be a way for you to
> > > provide a hint as to which addresses should be scanned (that's
> > > mii_bus::phy_mask) and possibly enhance that with a mask of possible
> > > C45 devices?
> >
> > Yes, i don't like this unconditional c45 scanning. A lot of MDIO bus
> > drivers don't look for the MII_ADDR_C45. They are going to do a C22
> > transfer, and maybe not mask out the MII_ADDR_C45 from reg, causing an
> > invalid register write. Bad things can then happen.
> >
> > With DT and ACPI, we have an explicit indication that C45 should be used,
> > so we know on this platform C45 is safe to use. We need something
> > similar when not using DT or ACPI.
> >
> > Andrew
>
> Florian and Andrew,
> The mdio c22 is using the start-of-frame ST=01 while mdio c45 is using ST=00
> as identifier. So mdio c22 device will not response to mdio c45 protocol.
> As in IEEE 802.1ae-2002 Annex 45A.3 mention that:
> " Even though the Clause 45 MDIO frames using the ST=00 frame code
> will also be driven on to the Clause 22 MII Management interface,
> the Clause 22 PHYs will ignore the frames. "
>
> Hence, I am not seeing any concern that the c45 scanning will mess up with
> c22 devices.
Hi Voon
Take for example mdio-hisi-femac.c
static int hisi_femac_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
{
struct hisi_femac_mdio_data *data = bus->priv;
int ret;
ret = hisi_femac_mdio_wait_ready(data);
if (ret)
return ret;
writel((mii_id << BIT_PHY_ADDR_OFFSET) | regnum,
data->membase + MDIO_RWCTRL);
There is no check here for MII_ADDR_C45. So it will perform a C22
transfer. And regnum will still have MII_ADDR_C45 in it, so the
writel() is going to set bit 30, since #define MII_ADDR_C45
(1<<30). What happens on this hardware under these conditions?
You cannot unconditionally ask an MDIO driver to do a C45
transfer. Some drivers are going to do bad things.
Andrew
^ permalink raw reply
* RE: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Parav Pandit @ 2019-08-27 16:13 UTC (permalink / raw)
To: Alex Williamson, Cornelia Huck
Cc: Jiri Pirko, kwankhede@nvidia.com, davem@davemloft.net,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <20190827092855.29702347@x1.home>
> -----Original Message-----
> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Tuesday, August 27, 2019 8:59 PM
> To: Cornelia Huck <cohuck@redhat.com>
> Cc: Parav Pandit <parav@mellanox.com>; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
>
> On Tue, 27 Aug 2019 13:29:46 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Tue, 27 Aug 2019 11:08:59 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all
> > > > mdevs
> > > >
> > > > On Mon, 26 Aug 2019 15:41:17 -0500 Parav Pandit
> > > > <parav@mellanox.com> wrote:
> > > >
> > > > > Mdev alias should be unique among all the mdevs, so that when
> > > > > such alias is used by the mdev users to derive other objects,
> > > > > there is no collision in a given system.
> > > > >
> > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > ---
> > > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > > 100644
> > > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> struct
> > > > device *dev,
> > > > > ret = -EEXIST;
> > > > > goto mdev_fail;
> > > > > }
> > > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > > >
> > > > Any way we can relay to the caller that the uuid was fine, but
> > > > that we had a hash collision? Duplicate uuids are much more obvious than
> a collision here.
> > > >
> > > How do you want to relay this rare event?
> > > Netlink interface has way to return the error message back, but sysfs is
> limited due to its error code based interface.
> >
> > I don't know, that's why I asked :)
> >
> > The problem is that "uuid already used" and "hash collision" are
> > indistinguishable. While "use a different uuid" will probably work in
> > both cases, "increase alias length" might be a good alternative in
> > some cases.
> >
> > But if there is no good way to relay the problem, we can live with it.
>
> It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\"
> for mdev device %pUl\n",...
>
Ok.
dev_dbg_once() to avoid message flood.
> Thanks,
> Alex
>
> > > > > + mutex_unlock(&mdev_list_lock);
> > > > > + ret = -EEXIST;
> > > > > + goto mdev_fail;
> > > > > + }
> > > > > }
> > > > >
> > > > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > >
> >
^ permalink raw reply
* RE: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Parav Pandit @ 2019-08-27 16:16 UTC (permalink / raw)
To: Alex Williamson
Cc: Mark Bloch, Jiri Pirko, kwankhede@nvidia.com, cohuck@redhat.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827092346.66bb73f1@x1.home>
> -----Original Message-----
> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Tuesday, August 27, 2019 8:54 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Mark Bloch <markb@mellanox.com>; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; cohuck@redhat.com; davem@davemloft.net;
> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
>
> On Tue, 27 Aug 2019 04:28:37 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > Hi Mark,
> >
> > > -----Original Message-----
> > > From: Mark Bloch <markb@mellanox.com>
> > > Sent: Tuesday, August 27, 2019 4:32 AM
> > > To: Parav Pandit <parav@mellanox.com>; alex.williamson@redhat.com;
> > > Jiri Pirko <jiri@mellanox.com>; kwankhede@nvidia.com;
> > > cohuck@redhat.com; davem@davemloft.net
> > > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > netdev@vger.kernel.org
> > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all
> > > mdevs
> > >
> > >
> > >
> > > On 8/26/19 1:41 PM, Parav Pandit wrote:
> > > > Mdev alias should be unique among all the mdevs, so that when such
> > > > alias is used by the mdev users to derive other objects, there is
> > > > no collision in a given system.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > ---
> > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > 100644
> > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> > > struct device *dev,
> > > > ret = -EEXIST;
> > > > goto mdev_fail;
> > > > }
> > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > >
> > > alias can be NULL here no?
> > >
> > If alias is NULL, tmp->alias would also be null because for given parent either
> we have alias or we don’t.
> > So its not possible to have tmp->alias as null and alias as non null.
> > But it may be good/defensive to add check for both.
>
> mdev_list is a global list of all mdev devices, how can we make any
> assumptions that an element has the same parent? Thanks,
>
Oh yes, right. If tmp->alias is not_null but alias can be NULL.
I will fix the check.
> Alex
>
> > > > + mutex_unlock(&mdev_list_lock);
> > > > + ret = -EEXIST;
> > > > + goto mdev_fail;
> > > > + }
> > > > }
> > > >
> > > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > > >
> > >
> > > Mark
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Alex Williamson @ 2019-08-27 16:24 UTC (permalink / raw)
To: Parav Pandit
Cc: Cornelia Huck, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB486671BB1CD562D070F0C0F2D1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 16:13:27 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Alex Williamson <alex.williamson@redhat.com>
> > Sent: Tuesday, August 27, 2019 8:59 PM
> > To: Cornelia Huck <cohuck@redhat.com>
> > Cc: Parav Pandit <parav@mellanox.com>; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> >
> > On Tue, 27 Aug 2019 13:29:46 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> >
> > > On Tue, 27 Aug 2019 11:08:59 +0000
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > > > To: Parav Pandit <parav@mellanox.com>
> > > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all
> > > > > mdevs
> > > > >
> > > > > On Mon, 26 Aug 2019 15:41:17 -0500 Parav Pandit
> > > > > <parav@mellanox.com> wrote:
> > > > >
> > > > > > Mdev alias should be unique among all the mdevs, so that when
> > > > > > such alias is used by the mdev users to derive other objects,
> > > > > > there is no collision in a given system.
> > > > > >
> > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > > ---
> > > > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > > > 1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > > > 100644
> > > > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> > struct
> > > > > device *dev,
> > > > > > ret = -EEXIST;
> > > > > > goto mdev_fail;
> > > > > > }
> > > > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > > > >
> > > > > Any way we can relay to the caller that the uuid was fine, but
> > > > > that we had a hash collision? Duplicate uuids are much more obvious than
> > a collision here.
> > > > >
> > > > How do you want to relay this rare event?
> > > > Netlink interface has way to return the error message back, but sysfs is
> > limited due to its error code based interface.
> > >
> > > I don't know, that's why I asked :)
> > >
> > > The problem is that "uuid already used" and "hash collision" are
> > > indistinguishable. While "use a different uuid" will probably work in
> > > both cases, "increase alias length" might be a good alternative in
> > > some cases.
> > >
> > > But if there is no good way to relay the problem, we can live with it.
> >
> > It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\"
> > for mdev device %pUl\n",...
> >
> Ok.
> dev_dbg_once() to avoid message flood.
I'd suggest a rate-limit rather than a once. The fact that the kernel
may have experienced a collision at some time in the past does not help
someone debug why they can't create a device now. The only way we're
going to get a flood is if a user sufficiently privileged to create
mdev devices stumbles onto a collision and continues to repeat the same
operation. That falls into shoot-yourself-in-the-foot behavior imo.
Thanks,
Alex
^ permalink raw reply
* [PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
From: Andy Shevchenko @ 2019-08-27 16:37 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi, ocfs2-devel, Ariel Elior,
Sudarsana Kalluru, GR-everest-linux-l2, David S. Miller, netdev,
Colin Ian King
Cc: Andy Shevchenko
There are users already and will be more of BITS_TO_BYTES() macro.
Move it to bitops.h for wider use.
In the case of ocfs2 the replacement is identical.
As for bnx2x, there are two places where floor version is used.
In the first case to calculate the amount of structures that can fit
one memory page. In this case obviously the ceiling variant is correct and
original code might have a potential bug, if amount of bits % 8 is not 0.
In the second case the macro is used to calculate bytes transmitted in one
microsecond. This will work for all speeds which is multiply of 1Gbps without
any change, for the rest new code will give ceiling value, for instance 100Mbps
will give 13 bytes, while old code gives 12 bytes and the arithmetically
correct one is 12.5 bytes. Further the value is used to setup timer threshold
which in any case has its own margins due to certain resolution. I don't see
here an issue with slightly shifting thresholds for low speed connections, the
card is supposed to utilize highest available rate, which is usually 10Gbps.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- described bnx2x cases in the commit message
- appended Rb (for ocfs2)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h | 1 -
fs/ocfs2/dlm/dlmcommon.h | 4 ----
include/linux/bitops.h | 1 +
3 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
index 066765fbef06..0a59a09ef82f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
@@ -296,7 +296,6 @@ static inline void bnx2x_dcb_config_qm(struct bnx2x *bp, enum cos_mode mode,
* possible, the driver should only write the valid vnics into the internal
* ram according to the appropriate port mode.
*/
-#define BITS_TO_BYTES(x) ((x)/8)
/* CMNG constants, as derived from system spec calculations */
diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h
index aaf24548b02a..0463dce65bb2 100644
--- a/fs/ocfs2/dlm/dlmcommon.h
+++ b/fs/ocfs2/dlm/dlmcommon.h
@@ -688,10 +688,6 @@ struct dlm_begin_reco
__be32 pad2;
};
-
-#define BITS_PER_BYTE 8
-#define BITS_TO_BYTES(bits) (((bits)+BITS_PER_BYTE-1)/BITS_PER_BYTE)
-
struct dlm_query_join_request
{
u8 node_idx;
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index cf074bce3eb3..79d80f5ddf7b 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -5,6 +5,7 @@
#include <linux/bits.h>
#define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
+#define BITS_TO_BYTES(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE)
#define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
extern unsigned int __sw_hweight8(unsigned int w);
--
2.23.0.rc1
^ permalink raw reply related
* [net-next 00/15][pull request] 100GbE Intel Wired LAN Driver Updates 2019-08-26
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann
This series contains updates to ice driver only.
Usha fixes the statistics reported on 4 port NICs which were reporting
the incorrect statistics due to using the incorrect port identifier.
Victor fixes an issue when trying to traverse to the first node of a
requested layer by adding a sibling head pointer for each layer per
traffic class.
Anirudh cleans up the locking and logic for enabling and disabling
VSI's to make it more consistent. Updates the driver to do dynamic
allocation of queue management bitmaps and arrays, rather than
statically allocating them which consumes more memory than required.
Refactor the logic in ice_ena_msix_range() for clarity and add
additional checks for when requested resources exceed what is available.
Jesse updates the debugging print statements to make it more useful when
dealing with link and PHY related issues.
Krzysztof adds a local variable to the VSI rebuild path to improve
readability.
Akeem limits the reporting of MDD events from VFs so that the kernel
log is not clogged up with MDD events which are duplicate or potentially
false positives. Fixed a reset issue that would result in the system
getting into a state that could only be resolved by a reboot by
testing if the VF is in a disabled state during a reset.
Michal adds a check to avoid trying to access memory that has not be
allocated by checking the number of queue pairs.
Jake fixes a static analysis warning due to a cast of a u8 to unsigned
long, so just update ice_is_tc_ena() to take a unsigned long so that a
cast is not necessary.
Colin Ian King fixes a potential infinite loop where a u8 is being
compared to an int.
Maciej refactors the queue handling functions that work on queue arrays
so that the logic can be done for a single queue.
Paul adds support for VFs to enable and disable single queues.
Henry fixed the order of operations in ice_remove() which was trying to
use adminq operations that were already disabled.
The following are changes since commit d00ee466a07eb9182ad3caf6140c7ebb527b4c64:
nfp: add AMDA0058 boards to firmware list
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE
Akeem G Abodunrin (2):
ice: Don't clog kernel debug log with VF MDD events errors
ice: Fix VF configuration issues due to reset
Anirudh Venkataramanan (3):
ice: Sanitize ice_ena_vsi and ice_dis_vsi
ice: Alloc queue management bitmaps and arrays dynamically
ice: Rework ice_ena_msix_range
Colin Ian King (1):
ice: fix potential infinite loop
Henry Tieman (1):
ice: fix adminq calls during remove
Jacob Keller (1):
ice: fix ice_is_tc_ena
Jesse Brandeburg (1):
ice: shorten local and add debug prints
Krzysztof Kazimierczak (1):
ice: Introduce a local variable for a VSI in the rebuild path
Maciej Fijalkowski (1):
ice: add support for enabling/disabling single queues
Michal Swiatkowski (1):
ice: add validation in OP_CONFIG_VSI_QUEUES VF message
Paul Greenwalt (1):
ice: add support for virtchnl_queue_select.[tx|rx]_queues bitmap
Usha Ketineni (1):
ice: Fix ethtool port and PFC stats for 4x25G cards
Victor Raj (1):
ice: added sibling head to parse nodes
drivers/net/ethernet/intel/ice/ice.h | 12 +-
drivers/net/ethernet/intel/ice/ice_common.c | 63 ++-
drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 13 +-
drivers/net/ethernet/intel/ice/ice_lib.c | 398 +++++++++++-------
drivers/net/ethernet/intel/ice/ice_lib.h | 28 +-
drivers/net/ethernet/intel/ice/ice_main.c | 205 +++++----
drivers/net/ethernet/intel/ice/ice_sched.c | 57 +--
drivers/net/ethernet/intel/ice/ice_type.h | 6 +-
.../net/ethernet/intel/ice/ice_virtchnl_pf.c | 279 ++++++++----
.../net/ethernet/intel/ice/ice_virtchnl_pf.h | 14 +-
10 files changed, 688 insertions(+), 387 deletions(-)
--
2.21.0
^ permalink raw reply
* [net-next 03/15] ice: Sanitize ice_ena_vsi and ice_dis_vsi
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem
Cc: Anirudh Venkataramanan, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190827163832.8362-1-jeffrey.t.kirsher@intel.com>
From: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
1. ndo_open and ndo_stop are implemented by ice_open and ice_stop
respectively. When enabling/disabling VSIs, just call
ice_open/ice_stop instead of ndo_open/ndo_stop.
2. Rework logic around rtnl_lock/rtnl_unlock
3. In ice_ena_vsi, remove an unnecessary stack variable and return
0 instead of err when __ICE_NEEDS_RESTART is not set.
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 24 +++++++++++------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 76a647cc2dbd..2f6125bfd991 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -436,13 +436,13 @@ static void ice_dis_vsi(struct ice_vsi *vsi, bool locked)
if (vsi->type == ICE_VSI_PF && vsi->netdev) {
if (netif_running(vsi->netdev)) {
- if (!locked) {
+ if (!locked)
rtnl_lock();
- vsi->netdev->netdev_ops->ndo_stop(vsi->netdev);
+
+ ice_stop(vsi->netdev);
+
+ if (!locked)
rtnl_unlock();
- } else {
- vsi->netdev->netdev_ops->ndo_stop(vsi->netdev);
- }
} else {
ice_vsi_close(vsi);
}
@@ -3654,21 +3654,19 @@ static int ice_ena_vsi(struct ice_vsi *vsi, bool locked)
int err = 0;
if (!test_bit(__ICE_NEEDS_RESTART, vsi->state))
- return err;
+ return 0;
clear_bit(__ICE_NEEDS_RESTART, vsi->state);
if (vsi->netdev && vsi->type == ICE_VSI_PF) {
- struct net_device *netd = vsi->netdev;
-
if (netif_running(vsi->netdev)) {
- if (locked) {
- err = netd->netdev_ops->ndo_open(netd);
- } else {
+ if (!locked)
rtnl_lock();
- err = netd->netdev_ops->ndo_open(netd);
+
+ err = ice_open(vsi->netdev);
+
+ if (!locked)
rtnl_unlock();
- }
}
}
--
2.21.0
^ permalink raw reply related
* [net-next 09/15] ice: fix potential infinite loop
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem
Cc: Colin Ian King, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190827163832.8362-1-jeffrey.t.kirsher@intel.com>
From: Colin Ian King <colin.king@canonical.com>
The loop counter of a for-loop is a u8 however this is being compared
to an int upper bound and this can lead to an infinite loop if the
upper bound is greater than 255 since the loop counter will wrap back
to zero. Fix this potential issue by making the loop counter an int.
Addresses-Coverity: ("Infinite loop")
Fixes: c7aeb4d1b9bf ("ice: Disable VFs until reset is completed")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 67cbebe1ff3f..0398c86226f0 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -477,7 +477,7 @@ static void
ice_prepare_for_reset(struct ice_pf *pf)
{
struct ice_hw *hw = &pf->hw;
- u8 i;
+ int i;
/* already prepared for reset */
if (test_bit(__ICE_PREPARED_FOR_RESET, pf->state))
--
2.21.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox