* [PATCH net] net: ethernet: ax88796: don't call free_irq without request_irq first
From: Uwe Kleine-König @ 2017-05-25 20:54 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, kernel, Ben Dooks
The function ax_init_dev (which is called only from the driver's .probe
function) calls free_irq in the error path without having requested the
irq in the first place. So drop the free_irq call in the error path.
Fixes: 825a2ff1896e ("AX88796 network driver")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/net/ethernet/8390/ax88796.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/8390/ax88796.c b/drivers/net/ethernet/8390/ax88796.c
index b0a3b85fc6f8..db02bc2fb4b2 100644
--- a/drivers/net/ethernet/8390/ax88796.c
+++ b/drivers/net/ethernet/8390/ax88796.c
@@ -748,13 +748,13 @@ static int ax_init_dev(struct net_device *dev)
ret = ax_mii_init(dev);
if (ret)
- goto out_irq;
+ goto err_out;
ax_NS8390_init(dev, 0);
ret = register_netdev(dev);
if (ret)
- goto out_irq;
+ goto err_out;
netdev_info(dev, "%dbit, irq %d, %lx, MAC: %pM\n",
ei_local->word16 ? 16 : 8, dev->irq, dev->base_addr,
@@ -762,9 +762,6 @@ static int ax_init_dev(struct net_device *dev)
return 0;
- out_irq:
- /* cleanup irq */
- free_irq(dev->irq, dev);
err_out:
return ret;
}
--
2.11.0
^ permalink raw reply related
* Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Jes Sorensen @ 2017-05-25 20:48 UTC (permalink / raw)
To: Saeed Mahameed, Ilan Tayari
Cc: Alexei Starovoitov, Saeed Mahameed, David S. Miller, Doug Ledford,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALzJLG-B_tAmASn_SMmPNiucq-tTpywHniRTkb4N32oGF6Y3Ng-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 05/25/2017 06:40 AM, Saeed Mahameed wrote:
> On Thu, May 25, 2017 at 8:20 AM, Ilan Tayari <ilant-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>>> -----Original Message-----
>>> Can you put it into different driver? Dumping everything into by far
>>> the biggest nic driver already is already huge headache in terms on
>>> maintainability, debugging and back ports.
>>> Look at how intel splits their drivers.
>>> ixgb, ixgbe, ixgbevf are different drivers thought they have a lot in
>>> common. On one side it's a bit of copy paste, but on the other side
>
> I don't think the ixgb example is the same, simply ixgb, ixgbe,
> ixgbevf have different PCI IDs
> and even different SW/FW interfaces. On the other hand, same mlx5
> driver can support all of
> ConnetX4/5/6 device IDs with the same code flows, same interfaces.
>
>>> it makes drivers much easier to develop and maintain independently.
>>> ConnectX-6 code and any future hw support doesn't belong to
>>> mlx5 driver at all.
>
> Sorry i must disagree with you on this for the same reasons Ilan mentioned.
> We can perfectly achieve the same with modular driver design all under the
> same .ko module, with some kconfig flags to reduce the amount of code/features
> this .ko provides.
If I get this right, the FPGA is independent and could in theory be used
for non network stuff. It really should have it's own driver in that
case, and you should provide accessor functionality via the mlx5 driver.
We have this with other devices in the kernel where a primary device
driver provides an interface for an additional sub-driver to access
another device behind it. Like bt-coexist in some of the wifi drivers
allowing access to a bluetooth device behind it.
Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net,v2] ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets
From: Peter Dawson @ 2017-05-25 20:35 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, stephen, netdev, linux-kernel
In-Reply-To: <20170523143616.38116e8f@gmail.com>
This fix addresses two problems in the way the DSCP field is formulated
on the encapsulating header of IPv6 tunnels.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=195661
1) The IPv6 tunneling code was manipulating the DSCP field of the
encapsulating packet using the 32b flowlabel. Since the flowlabel is
only the lower 20b it was incorrect to assume that the upper 12b
containing the DSCP and ECN fields would remain intact when formulating
the encapsulating header. This fix handles the 'inherit' and
'fixed-value' DSCP cases explicitly using the extant dsfield u8 variable.
2) The use of INET_ECN_encapsulate(0, dsfield) in ip6_tnl_xmit was
incorrect and resulted in the DSCP value always being set to 0.
Commit 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class
is non-0") caused the regression by masking out the flowlabel
which exposed the incorrect handling of the DSCP portion of the
flowlabel in ip6_tunnel and ip6_gre.
Fixes: 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class is non-0")
Signed-off-by: Peter Dawson <peter.a.dawson@boeing.com>
---
net/ipv6/ip6_gre.c | 13 +++++++------
net/ipv6/ip6_tunnel.c | 21 +++++++++++++--------
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 8d128ba..0c5b4caa 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -537,11 +537,10 @@ static inline int ip6gre_xmit_ipv4(struct sk_buff *skb, struct net_device *dev)
memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
- dsfield = ipv4_get_dsfield(iph);
-
if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
- fl6.flowlabel |= htonl((__u32)iph->tos << IPV6_TCLASS_SHIFT)
- & IPV6_TCLASS_MASK;
+ dsfield = ipv4_get_dsfield(iph);
+ else
+ dsfield = ip6_tclass(t->parms.flowinfo);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
fl6.flowi6_mark = skb->mark;
else
@@ -598,9 +597,11 @@ static inline int ip6gre_xmit_ipv6(struct sk_buff *skb, struct net_device *dev)
memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
- dsfield = ipv6_get_dsfield(ipv6h);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
- fl6.flowlabel |= (*(__be32 *) ipv6h & IPV6_TCLASS_MASK);
+ dsfield = ipv6_get_dsfield(ipv6h);
+ else
+ dsfield = ip6_tclass(t->parms.flowinfo);
+
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FLOWLABEL)
fl6.flowlabel |= ip6_flowlabel(ipv6h);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 6eb2ae5..7ae6c50 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1196,7 +1196,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield,
skb_push(skb, sizeof(struct ipv6hdr));
skb_reset_network_header(skb);
ipv6h = ipv6_hdr(skb);
- ip6_flow_hdr(ipv6h, INET_ECN_encapsulate(0, dsfield),
+ ip6_flow_hdr(ipv6h, dsfield,
ip6_make_flowlabel(net, skb, fl6->flowlabel, true, fl6));
ipv6h->hop_limit = hop_limit;
ipv6h->nexthdr = proto;
@@ -1231,8 +1231,6 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
if (tproto != IPPROTO_IPIP && tproto != 0)
return -1;
- dsfield = ipv4_get_dsfield(iph);
-
if (t->parms.collect_md) {
struct ip_tunnel_info *tun_info;
const struct ip_tunnel_key *key;
@@ -1246,6 +1244,7 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
fl6.flowi6_proto = IPPROTO_IPIP;
fl6.daddr = key->u.ipv6.dst;
fl6.flowlabel = key->label;
+ dsfield = ip6_tclass(key->label);
} else {
if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
encap_limit = t->parms.encap_limit;
@@ -1254,8 +1253,9 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
fl6.flowi6_proto = IPPROTO_IPIP;
if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
- fl6.flowlabel |= htonl((__u32)iph->tos << IPV6_TCLASS_SHIFT)
- & IPV6_TCLASS_MASK;
+ dsfield = ipv4_get_dsfield(iph);
+ else
+ dsfield = ip6_tclass(t->parms.flowinfo);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
fl6.flowi6_mark = skb->mark;
else
@@ -1267,6 +1267,8 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP6))
return -1;
+ dsfield = INET_ECN_encapsulate(dsfield, ipv4_get_dsfield(iph));
+
skb_set_inner_ipproto(skb, IPPROTO_IPIP);
err = ip6_tnl_xmit(skb, dev, dsfield, &fl6, encap_limit, &mtu,
@@ -1300,8 +1302,6 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
ip6_tnl_addr_conflict(t, ipv6h))
return -1;
- dsfield = ipv6_get_dsfield(ipv6h);
-
if (t->parms.collect_md) {
struct ip_tunnel_info *tun_info;
const struct ip_tunnel_key *key;
@@ -1315,6 +1315,7 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
fl6.flowi6_proto = IPPROTO_IPV6;
fl6.daddr = key->u.ipv6.dst;
fl6.flowlabel = key->label;
+ dsfield = ip6_tclass(key->label);
} else {
offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb));
/* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */
@@ -1337,7 +1338,9 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
fl6.flowi6_proto = IPPROTO_IPV6;
if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
- fl6.flowlabel |= (*(__be32 *)ipv6h & IPV6_TCLASS_MASK);
+ dsfield = ipv6_get_dsfield(ipv6h);
+ else
+ dsfield = ip6_tclass(t->parms.flowinfo);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FLOWLABEL)
fl6.flowlabel |= ip6_flowlabel(ipv6h);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
@@ -1351,6 +1354,8 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP6))
return -1;
+ dsfield = INET_ECN_encapsulate(dsfield, ipv6_get_dsfield(ipv6h));
+
skb_set_inner_ipproto(skb, IPPROTO_IPV6);
err = ip6_tnl_xmit(skb, dev, dsfield, &fl6, encap_limit, &mtu,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets
From: David Miller @ 2017-05-25 20:33 UTC (permalink / raw)
To: petedaws; +Cc: kuznet, jmorris, yoshfuji, kaber, stephen, netdev, linux-kernel
In-Reply-To: <20170526060842.4bd21054@udesktop>
From: Peter Dawson <petedaws@gmail.com>
Date: Fri, 26 May 2017 06:08:42 +1000
> On Thu, 25 May 2017 15:49:14 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
>
>> Still not correct, you need to use a "Fixes: " tag of the form:
>>
>> Fixes: 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class is non-0")
>>
>> And it must appear of the first line of tags, before signoffs and acks,
>> with no empty lines in between.
>
> Here is my proposed commit message in full. Note that
> the "Fixes" line extends to 78 chars. Is this OK?
Yes, that looks perfect.
^ permalink raw reply
* Re: [PATCH net] sky2: Do not deadlock on sky2_hw_down
From: Joshua Emele @ 2017-05-25 20:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Joshua Emele, Mirko Lindner, linux-kernel
In-Reply-To: <20170525105458.539dedba@xeon-e3>
On Thu, May 25, 2017 at 10:54 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Wed, 24 May 2017 15:43:18 -0700
> Joshua Emele <jemele@gmail.com> wrote:
>
> > From: Joshua Emele <jemele@google.com>
> >
> > The sky2_hw_down uses sky2_tx_complete to free pending frames stuck in
> > the HW queue. Because sky2_hw_down can be called from a process context,
> > the call to u64_stats_update_begin can result in deadlock.
> >
> > Because the statistics do not require update as part of the sky2_hw_down
> > sequence, prevent the update to avoid the deadlock.
> >
> > [18198.003900] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> > [18198.009931] ifconfig/11604 [HC0[0]:SC0[0]:HE1:SE1] takes:
> > [18198.015348] (&syncp->seq#2){+.?...}, at: [<805ed440>] sky2_hw_down+0x370/0x428
> > [18198.022827] {IN-SOFTIRQ-W} state was registered at:
> > [18198.027723] [<801729a0>] lock_acquire+0x78/0x98
> > [18198.032484] [<805ed048>] sky2_tx_complete+0x1a8/0x230
> > [18198.037760] [<805f2718>] sky2_poll+0x7cc/0xfa8
> > [18198.042425] [<807692f8>] net_rx_action+0x200/0x330
> > [18198.047447] [<80129f64>] __do_softirq+0x130/0x31c
> > [18198.052369] [<8012a4a0>] irq_exit+0xc8/0x13c
> > [18198.056836] [<8017e398>] __handle_domain_irq+0x84/0xf8
> > [18198.062180] [<80101564>] gic_handle_irq+0x58/0xb8
> > [18198.067086] [<8010d838>] __irq_usr+0x58/0x80
> > [18198.071557] [<76e178ae>] 0x76e178ae
> > [18198.075247] irq event stamp: 2109
> > [18198.078568] hardirqs last enabled at (2109): [<8012a2cc>] __local_bh_enable_ip+0x90/0x110
> > [18198.086860] hardirqs last disabled at (2107): [<8012a27c>] __local_bh_enable_ip+0x40/0x110
> > [18198.095150] softirqs last enabled at (2108): [<805ed368>] sky2_hw_down+0x298/0x428
> > [18198.102832] softirqs last disabled at (2106): [<805ed284>] sky2_hw_down+0x1b4/0x428
> > [18198.110515]
> > other info that might help us debug this:
> > [18198.117048] Possible unsafe locking scenario:
> >
> > [18198.122974] CPU0
> > [18198.125425] ----
> > [18198.127876] lock(&syncp->seq#2);
> > [18198.131332] <Interrupt>
> > [18198.133955] lock(&syncp->seq#2);
> > [18198.137585]
> > *** DEADLOCK ***
> >
> > [18198.143517] 1 lock held by ifconfig/11604:
> > [18198.147618] #0: (rtnl_mutex){+.+.+.}, at: [<8077e874>] rtnl_lock+0x18/0x20
> > [18198.154772]
> > stack backtrace:
> > [18198.159143] CPU: 1 PID: 11604 Comm: ifconfig Not tainted 4.8.17 #1
> > [18198.165331] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> > [18198.171864] Backtrace:
> > [18198.174353] [<8010c634>] (dump_backtrace) from [<8010c828>] (show_stack+0x18/0x1c)
> > [18198.181931] r6:60070093 r5:00000000 r4:80e23248 r3:00000000
> > [18198.187682] [<8010c810>] (show_stack) from [<8041edc4>] (dump_stack+0xb4/0xe8)
> > [18198.194924] [<8041ed10>] (dump_stack) from [<801700cc>] (print_usage_bug+0x1dc/0x2d0)
> > [18198.202762] r10:edec55c0 r9:80bd1178 r8:81649598 r7:00000006 r6:edec5a90 r5:80fb09f4
> > [18198.210686] r4:edec55c0 r3:00000002
> > [18198.214311] [<8016fef0>] (print_usage_bug) from [<80170348>] (mark_lock+0x188/0x6c4)
> > [18198.222059] r9:00000000 r8:00000004 r7:edec55c0 r6:00000006 r5:edec5a90 r4:8016f44c
> > [18198.229903] [<801701c0>] (mark_lock) from [<801714ac>] (__lock_acquire+0xb90/0x1c70)
> > [18198.237653] r10:edec55c0 r9:00000000 r8:edec5aa4 r7:00000004 r6:00000001 r5:00000000
> > [18198.245573] r4:000001cd r3:00000001
> > [18198.249192] [<8017091c>] (__lock_acquire) from [<801729a0>] (lock_acquire+0x78/0x98)
> > [18198.256940] r10:00000001 r9:000006d0 r8:00000000 r7:00000001 r6:805ed440 r5:60070013
> > [18198.264860] r4:00000000
> > [18198.267428] [<80172928>] (lock_acquire) from [<805ed048>] (sky2_tx_complete+0x1a8/0x230)
> > [18198.275523] r7:ee2d9dfc r6:00000000 r5:ee2d9dc0 r4:00000000
> > [18198.281266] [<805ecea0>] (sky2_tx_complete) from [<805ed440>] (sky2_hw_down+0x370/0x428)
> > [18198.289360] r10:ee2cac00 r9:000006d0 r8:ee2d9dc0 r7:f0d40aa8 r6:00000001 r5:00000d48
> > [18198.297281] r4:00000000
> > [18198.299850] [<805ed0d0>] (sky2_hw_down) from [<805efed8>] (sky2_close+0xac/0x11c)
> > [18198.307337] r10:ee838600 r9:00000000 r8:00000000 r7:00001003 r6:ee2d9dc0 r5:00000000
> > [18198.315258] r4:ee2cac00
> > [18198.317830] [<805efe2c>] (sky2_close) from [<8076a310>] (__dev_close_many+0xc4/0x118)
> > [18198.325664] r7:00001003 r6:00001042 r5:eb047df8 r4:ee2d9800
> > [18198.331404] [<8076a24c>] (__dev_close_many) from [<8076ab24>] (__dev_close+0x30/0x48)
> > [18198.339240] r5:00000001 r4:ee2d9800
> > [18198.342866] [<8076aaf4>] (__dev_close) from [<8076dbac>] (__dev_change_flags+0x90/0x154)
> > [18198.350970] [<8076db1c>] (__dev_change_flags) from [<8076dc90>] (dev_change_flags+0x20/0x50)
> > [18198.359412] r8:00000000 r7:00008914 r6:00001003 r5:ee2d9948 r4:ee2d9800 r3:00000014
> > [18198.367264] [<8076dc70>] (dev_change_flags) from [<807f06d0>] (devinet_ioctl+0x724/0x818)
> > [18198.375448] r8:ffffffff r7:00008914 r6:ee2cae0c r5:7ed799ac r4:eb047e80 r3:00000014
> > [18198.383291] [<807effac>] (devinet_ioctl) from [<807f2e98>] (inet_ioctl+0x19c/0x1c8)
> > [18198.390951] r10:edfc3b80 r9:eb046000 r8:00000004 r7:ee724b40 r6:7ed799ac r5:ee724b60
> > [18198.398872] r4:00008914
> > [18198.401446] [<807f2cfc>] (inet_ioctl) from [<8074a234>] (sock_ioctl+0x158/0x300)
> > [18198.408860] [<8074a0dc>] (sock_ioctl) from [<80239770>] (do_vfs_ioctl+0x98/0xa3c)
> > [18198.416348] r7:8023a150 r6:00000004 r5:ee724b60 r4:7ed799ac
> > [18198.422087] [<802396d8>] (do_vfs_ioctl) from [<8023a150>] (SyS_ioctl+0x3c/0x64)
> > [18198.429401] r10:00000000 r9:eb046000 r8:00000004 r7:00008914 r6:edfc3b80 r5:7ed799ac
> > [18198.437326] r4:edfc3b80
> > [18198.439895] [<8023a114>] (SyS_ioctl) from [<80107ee0>] (ret_fast_syscall+0x0/0x1c)
> > [18198.447469] r8:80108084 r7:00000036 r6:00000004 r5:7ed799ac r4:7ed79b20 r3:31687465
> >
> > Signed-off-by: Joshua Emele <jemele@google.com>
>
> This problem was introduced by the "better version of seqcount". The original
> version which is the raw_XXX version does not have any locking.
>
> Sigh. The point of u64_stats_update_begin/end was that they are supposed to be lock less.
> What architecture are you using that has such a broken version which uses locks?
>
I'm using a Gatework 5304 Ventana, which is an imx6q
(CONFIG_SOC_IMX6Q) single board computer (
http://trac.gateworks.com/wiki/ventana).
^ permalink raw reply
* Re: [PATCH net] ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets
From: Peter Dawson @ 2017-05-25 20:08 UTC (permalink / raw)
To: David Miller
Cc: kuznet, jmorris, yoshfuji, kaber, stephen, netdev, linux-kernel
In-Reply-To: <20170525.154914.1347048861561693629.davem@davemloft.net>
On Thu, 25 May 2017 15:49:14 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> Still not correct, you need to use a "Fixes: " tag of the form:
>
> Fixes: 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class is non-0")
>
> And it must appear of the first line of tags, before signoffs and acks,
> with no empty lines in between.
Here is my proposed commit message in full. Note that
the "Fixes" line extends to 78 chars. Is this OK?
Thanks
This fix addresses two problems in the way the DSCP field is formulated
on the encapsulating header of IPv6 tunnels.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=195661
1) The IPv6 tunneling code was manipulating the DSCP field of the
encapsulating packet using the 32b flowlabel. Since the flowlabel is
only the lower 20b it was incorrect to assume that the upper 12b
containing the DSCP and ECN fields would remain intact when formulating
the encapsulating header. This fix handles the 'inherit' and
'fixed-value' DSCP cases explicitly using the extant dsfield u8 variable.
2) The use of INET_ECN_encapsulate(0, dsfield) in ip6_tnl_xmit was
incorrect and resulted in the DSCP value always being set to 0.
Commit 90427ef5d2a4 caused the regression by masking out the flowlabel
which exposed the incorrect the handling of the DSCP portion of the
flowlabel in ip6_tunnel and ip6_gre.
Fixes: 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class is non-0")
Signed-off-by: Peter Dawson <peter.a.dawson@boeing.com>
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Alexander Duyck @ 2017-05-25 19:49 UTC (permalink / raw)
To: Ding Tianhong
Cc: Casey Leedom, Raj, Ashok, Bjorn Helgaas, Michael Werner,
Ganesh GR, Arjun V., Asit K Mallick, Patrick J Cramer,
Suravee Suthikulpanit, Bob Shaw, h, Mark Rutland, Amir Ancel,
Gabriele Paoloni, Catalin Marinas, Will Deacon, LinuxArm,
David Laight, Jeff Kirsher, Netdev
In-Reply-To: <2eab1a1d-3444-fe11-7626-5b6459e954f8@huawei.com>
On Thu, May 25, 2017 at 6:35 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
>
> On 2017/5/9 8:48, Casey Leedom wrote:
>>
>> | From: Alexander Duyck <alexander.duyck@gmail.com>
>> | Date: Saturday, May 6, 2017 11:07 AM
>> |
>> | | From: Ding Tianhong <dingtianhong@huawei.com>
>> | | Date: Fri, May 5, 2017 at 8:08 PM
>> | |
>> | | According the suggestion, I could only think of this code:
>> | | ..
>> |
>> | This is a bit simplistic but it is a start.
>>
>> Yes, something tells me that this is going to be more complicated than any
>> of us like ...
>>
>> | The other bit I was getting at is that we need to update the core PCIe
>> | code so that when we configure devices and the root complex reports no
>> | support for relaxed ordering it should be clearing the relaxed
>> | ordering bits in the PCIe configuration registers on the upstream
>> | facing devices.
>>
>> Of course, this can be written to by the Driver at any time ... and is in
>> the case of the cxgb4 Driver ...
>>
>> After a lot of rummaging around, it does look like KVM prohibits writes to
>> the PCIe Capability Device Control register in drivers/xen/xen-pciback/
>> conf_space_capability.c and conf_space.c simply because writes aren't
>> allowed unless "permissive" is set. So it ~looks~ like a driver running in
>> a Virtual Machine can't turn Enable Relaxed Ordering back on ...
>>
>> | The last bit we need in all this is a way to allow for setups where
>> | peer-to-peer wants to perform relaxed ordering but for writes to the
>> | host we have to not use relaxed ordering. For that we need to enable a
>> | special case and that isn't handled right now in any of the solutions
>> | we have coded up so far.
>>
>> Yes, we do need this.
>>
>>
>> | From: Alexander Duyck <alexander.duyck@gmail.com>
>> | Date: Saturday, May 8, 2017 08:22 AM
>> |
>> | The problem is we need to have something that can be communicated
>> | through a VM. Your change doesn't work in that regard. That was why I
>> | suggested just updating the code so that we when we initialized PCIe
>> | devices what we do is either set or clear the relaxed ordering bit in
>> | the PCIe device control register. That way when we direct assign an
>> | interface it could know just based on the bits int the PCIe
>> | configuration if it could use relaxed ordering or not.
>> |
>> | At that point the driver code itself becomes very simple since you
>> | could just enable the relaxed ordering by default in the igb/ixgbe
>> | driver and if the bit is set or cleared in the PCIe configuration then
>> | we are either sending with relaxed ordering requests or not and don't
>> | have to try and locate the root complex.
>> |
>> | So from the sound of it Casey has a special use case where he doesn't
>> | want to send relaxed ordering frames to the root complex, but instead
>> | would like to send them to another PCIe device. To do that he needs to
>> | have a way to enable the relaxed ordering bit in the PCIe
>> | configuration but then not send any to the root complex. Odds are that
>> | is something he might be able to just implement in the driver, but is
>> | something that may become a more general case in the future. I don't
>> | see our change here impacting it as long as we keep the solution
>> | generic and mostly confined to when we instantiate the devices as the
>> | driver could likely make the decision to change the behavior later.
>>
>> It's not just me. Intel has said that while RO directed at the Root
>> Complex Host Coherent Memory has a performance bug (not Data Corruption),
>> it's a performance win for Peer-to-Peer writes to MMIO Space. (I'll be very
>> interested in hearing what the bug is if we get that much detail. The very
>> same TLPs directed to the Root Complex Port without Relaxed Ordering set get
>> good performance. So this is essentially a bug in the hardware that was
>> ~trying~ to implement a performance win.)
>>
>> Meanwhile, I currently only know of a single PCIe End Point which causes
>> catastrophic results: the AMD A1100 ARM SoC ("SEATTLE"). And it's not even
>> clear that product is even alive anymore since I haven't been able to get
>> any responses from them for several months.
>>
>> What I'm saying is: let's try to architect a solution which doesn't throw
>> the baby out with the bath water ...
>>
>> I think that if a Device's Root Complex Port has problems with Relaxed
>> Ordering, it ~probably~ makes sense to turn off the PCIe Capability Device
>> Control[Enable Relaxed Ordering] when we assign a device to a Virtual
>> Machine since the Device Driver can no longer query the Relaxed Ordering
>> Support of the Root Complex Port. The only down side of this would be if we
>> assigned two Peers to a VM in an application which wanted to do Peer-to-Peer
>> transfers. But that seems like a hard application to support in any case
>> since the PCI Bus:Slot.Function IDs for assigned Devices within a VM don't
>> match the actual values.
>>
>> For Devices running in the base OS/Hypervisor, their Drivers can query the
>> Relaxed Ordering Support for the Root Complex Port or a Peer Device. So a
>> simple flag within the (struct pci_dev *)->dev_flags would serve for that
>> along with a per-Architecture/Platform mechanism for setting it ...
>>
>> Casey
>>
>
> I have take a time to talk to our kvm team about how to distinguish the relaxed
> ordering in the VM for some vf just like 82599-vf, the probe routine looks like
> could work like this:
> 1) QEMU could emulate the platform by the Vender ID and device ID which could be
> read from the host.
> 2) The QEMU could create a virtual PCIe dev complex and recognize the PCIe bus address which
> come and detach from the host to the guest.
> 3) the PCI quirk could enable the Relaxed Ordering by the Vendor ID and Device ID.
> 4) The VF drivers could read the flag and set to the hw.
>
> So I think we could set the PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED for some special platform
> and don't enable by default, if I miss something, please not hesitate to enlighten me :)
This isn't what I had in mind at all.
So what Casey had originally submitted was a step in the direction of
what I was thinking. Basically on platforms where it is not advisable
to enable relaxed ordering we need to advertise that relaxed ordering
is not safe. Then when we are initializing the devices underneath
those we need to be clearing the relaxed ordering enable bits in the
PCI configuration, that is the piece that was missing from Casey's
original patch. In addition we then need to have a way for devices to
optionally enable relaxed ordering for cases like Casey pointed out
where they might want to use relaxed ordering for peer-to-peer
transactions, but not for transactions to the root complex. Finally in
the case of the Intel drivers we could then just drop the compile time
checks entirely and just enable the device to configure the internal
registers for relaxed ordering because the configuration space becomes
the spot that controls if this gets enabled or not.
So the initial set of patches Casey submitted only really consisted of
2 patches. What I am proposing is that we would be looking at
expanding this out to about 4 patches. The first patch is the original
1 of 2, the second patch would be to modify the PCI initialization
code to clear the relaxed ordering enable bit in the event that the
root complex doesn't want relaxed ordering enabled, the third would be
to make changes to the Chelsio driver as needed to allow for the
peer-to-peer case to be enabled when the relaxed ordering bit in the
configuration space is not enabled without triggering any relaxed
ordering requests to the root complex, and the last one would be to
drop the defines in ixgbe and whatever other Intel drivers are
currently checking for either SPARC or the define that was added to
support relaxed ordering and just act like we are going to do it
always with the PCI configuration space controlling if we do or not.
Ideally as a part of the second patch we should have a way of testing
if a given path can support relaxed ordering. That way when we go to
try to enable a peer-to-peer setup we can be certain that a given path
will work and don't try enabling it in paths that would be unsupported
for peer-to-peer.
This ends up being somewhat of a risk for the Intel NICs, but if the
Chelsio devices have been running with relaxed ordering enabled for
some time and have identified the chipsets that should be issues, then
odds are we should be fine as well.
> --------------------------------------------------------------
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 085fb78..74bcc25 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4664,3 +4664,22 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
> +
> +/*
> + * Some devices have problems with Transaction Layer Packets with the Relaxed
> + * Ordering Attribute set, so we should disable Relaxed Ordering by default
> + * and only enable it when some devices has mark themselves and other
> + * Device Drivers should check before sending TLPs with RO set.
> + */
> +static void quirk_relaxedordering_enable(struct pci_dev *dev)
> +{
> + dev->dev_flags &= ~PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED;
> +}
> +
> +/*
> + * Hisilicon Root Complex could support relaxed ordering which can
> + * improve performance with Upstream Transaction Layer Packets with
> + * Relaxed Ordering set.
> + */
> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_HUAWEI, 0x1610, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_enable);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 33c2b0b..f7d8d6f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -183,6 +183,8 @@ enum pci_dev_flags {
> PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT = (__force pci_dev_flags_t) (1 << 9),
> /* Do not use FLR even if device advertises PCI_AF_CAP */
> PCI_DEV_FLAGS_NO_FLR_RESET = (__force pci_dev_flags_t) (1 << 10),
> + /* Use Relaxed Ordering for TLPs directed at this device */
> + PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED = (__force pci_dev_flags_t) (1 << 11),
> };
>
> enum pci_irq_reroute_variant {
> @@ -2203,6 +2205,20 @@ static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
> return false;
> }
>
> +/**
> + * pci_is_dev_relaxed_ordering_enabled - whether device could support Relaxed
> + * Ordering for TLPs directed.
> + * @pdev: PCI device to check
> + *
> + * This function could return the value indicates that whether Relaxed Ordering
> + * Attribute could be used on Transaction Layer Packets destined for the PCIe
> + * End Node.
> + */
> +static inline boot pci_is_dev_relaxed_ordering_enabled(struct pci_dev *pdev)
> +{
> + return (pdev->dev_flags & PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED) ==
> + PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED;
> /* provide the legacy pci_dma_* API */
> #include <linux/pci-dma-compat.h>
>
> Thanks
> Ding
>
>> .
>>
>
^ permalink raw reply
* Re: [PATCH net] ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets
From: David Miller @ 2017-05-25 19:49 UTC (permalink / raw)
To: petedaws; +Cc: kuznet, jmorris, yoshfuji, kaber, stephen, netdev, linux-kernel
In-Reply-To: <20170526054627.00dcdeeb@udesktop>
From: Peter Dawson <petedaws@gmail.com>
Date: Fri, 26 May 2017 05:46:27 +1000
> On Thu, 25 May 2017 12:11:17 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
>
>> > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=195661
>>
>> This is not the correct way to use the Fixes: tag.
>>
>> You should specify the commit that introduced the regression
>> between 4.9.x and 4.10.x, and that you are fixing here.
>
> Thanks for your review Dave. I'll resubmit the patch with the following detail and just reference the bugzilla in the main body of the commit comment.
>
> Commit 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class is non-0") caused the regression by masking out the flowlabel which exposed the incorrect the handling of the DSCP portion of the flowlabel in ip6_tunnel and ip6_gre.
Still not correct, you need to use a "Fixes: " tag of the form:
Fixes: 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class is non-0")
And it must appear of the first line of tags, before signoffs and acks,
with no empty lines in between.
Thank you.
^ permalink raw reply
* Re: [PATCH net] ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets
From: Peter Dawson @ 2017-05-25 19:46 UTC (permalink / raw)
To: David Miller
Cc: kuznet, jmorris, yoshfuji, kaber, stephen, netdev, linux-kernel
In-Reply-To: <20170525.121117.810915705354733044.davem@davemloft.net>
On Thu, 25 May 2017 12:11:17 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=195661
>
> This is not the correct way to use the Fixes: tag.
>
> You should specify the commit that introduced the regression
> between 4.9.x and 4.10.x, and that you are fixing here.
Thanks for your review Dave. I'll resubmit the patch with the following detail and just reference the bugzilla in the main body of the commit comment.
Commit 90427ef5d2a4 ("ipv6: fix flow labels when the traffic class is non-0") caused the regression by masking out the flowlabel which exposed the incorrect the handling of the DSCP portion of the flowlabel in ip6_tunnel and ip6_gre.
^ permalink raw reply
* Re: [PATCH net-next 6/6] net: mpls: minor cleanups
From: Joe Perches @ 2017-05-25 19:46 UTC (permalink / raw)
To: David Miller, dsahern; +Cc: netdev, roopa
In-Reply-To: <20170525.145631.1427287907712541949.davem@davemloft.net>
On Thu, 2017-05-25 at 14:56 -0400, David Miller wrote:
> From: David Ahern <dsahern@gmail.com>
> Date: Wed, 24 May 2017 21:54:42 -0600
>
> > Noticed these doing the extack support:
> > - nla_get_via is only used in af_mpls.c so remove from internal.h
> ...
> > @@ -43,6 +43,9 @@ static void rtmsg_lfib(int event, u32 label, struct mpls_route *rt,
> > struct nlmsghdr *nlh, struct net *net, u32 portid,
> > unsigned int nlm_flags);
> >
> > +static int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
> > + u8 via[], struct netlink_ext_ack *extack);
> > +
> > static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
> > {
> > struct mpls_route *rt = NULL;
>
> David, please mark the actual defintion static as well.
>
> I'm surprised the compiler accepts this, but it does :-)
As well, the function should be reordered
in the file to avoid the prototype.
Something like:
---
net/mpls/af_mpls.c | 84 ++++++++++++++++++++++++++---------------------------
net/mpls/internal.h | 2 --
2 files changed, 42 insertions(+), 44 deletions(-)
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 257ec66009da..9b534dc25b9b 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -718,6 +718,48 @@ static int mpls_nh_build_from_cfg(struct mpls_route_config *cfg,
return err;
}
+static int nla_get_via(const struct nlattr *nla, u8 *via_alen,
+ u8 *via_table, u8 via_addr[])
+{
+ struct rtvia *via = nla_data(nla);
+ int err = -EINVAL;
+ int alen;
+
+ if (nla_len(nla) < offsetof(struct rtvia, rtvia_addr))
+ goto errout;
+ alen = nla_len(nla) -
+ offsetof(struct rtvia, rtvia_addr);
+ if (alen > MAX_VIA_ALEN)
+ goto errout;
+
+ /* Validate the address family */
+ switch (via->rtvia_family) {
+ case AF_PACKET:
+ *via_table = NEIGH_LINK_TABLE;
+ break;
+ case AF_INET:
+ *via_table = NEIGH_ARP_TABLE;
+ if (alen != 4)
+ goto errout;
+ break;
+ case AF_INET6:
+ *via_table = NEIGH_ND_TABLE;
+ if (alen != 16)
+ goto errout;
+ break;
+ default:
+ /* Unsupported address family */
+ goto errout;
+ }
+
+ memcpy(via_addr, via->rtvia_addr, alen);
+ *via_alen = alen;
+ err = 0;
+
+errout:
+ return err;
+}
+
static int mpls_nh_build(struct net *net, struct mpls_route *rt,
struct mpls_nh *nh, int oif, struct nlattr *via,
struct nlattr *newdst, u8 max_labels)
@@ -1594,48 +1636,6 @@ int nla_get_labels(const struct nlattr *nla,
}
EXPORT_SYMBOL_GPL(nla_get_labels);
-int nla_get_via(const struct nlattr *nla, u8 *via_alen,
- u8 *via_table, u8 via_addr[])
-{
- struct rtvia *via = nla_data(nla);
- int err = -EINVAL;
- int alen;
-
- if (nla_len(nla) < offsetof(struct rtvia, rtvia_addr))
- goto errout;
- alen = nla_len(nla) -
- offsetof(struct rtvia, rtvia_addr);
- if (alen > MAX_VIA_ALEN)
- goto errout;
-
- /* Validate the address family */
- switch (via->rtvia_family) {
- case AF_PACKET:
- *via_table = NEIGH_LINK_TABLE;
- break;
- case AF_INET:
- *via_table = NEIGH_ARP_TABLE;
- if (alen != 4)
- goto errout;
- break;
- case AF_INET6:
- *via_table = NEIGH_ND_TABLE;
- if (alen != 16)
- goto errout;
- break;
- default:
- /* Unsupported address family */
- goto errout;
- }
-
- memcpy(via_addr, via->rtvia_addr, alen);
- *via_alen = alen;
- err = 0;
-
-errout:
- return err;
-}
-
static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh,
struct mpls_route_config *cfg)
{
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index 4db6a5971322..b431c6d3893f 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -204,8 +204,6 @@ int nla_put_labels(struct sk_buff *skb, int attrtype, u8 labels,
const u32 label[]);
int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
u32 label[]);
-int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
- u8 via[]);
bool mpls_output_possible(const struct net_device *dev);
unsigned int mpls_dev_mtu(const struct net_device *dev);
bool mpls_pkt_too_big(const struct sk_buff *skb, unsigned int mtu);
^ permalink raw reply related
* [PATCH v2 net-next 1/4] net: phy: marvell: #defines for copper and fibre pages
From: Andrew Lunn @ 2017-05-25 19:42 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495741328-25001-1-git-send-email-andrew@lunn.ch>
Replace magic numbers for PHY pages with symbolic names.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 88cd97b44ba6..bb067026353a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -650,7 +650,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
mdelay(500);
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -662,7 +662,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- err = marvell_set_page(phydev, 2);
+ err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
if (err < 0)
return err;
temp = phy_read(phydev, MII_M1116R_CONTROL_REG_MAC);
@@ -671,7 +671,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
err = phy_write(phydev, MII_M1116R_CONTROL_REG_MAC, temp);
if (err < 0)
return err;
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -892,7 +892,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
return err;
/* Reset page selection */
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
}
@@ -922,7 +922,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, 2);
+ err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
if (err < 0)
return err;
@@ -932,7 +932,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Change address */
- err = marvell_set_page(phydev, 3);
+ err = marvell_set_page(phydev, MII_88E1318S_PHY_LED_PAGE);
if (err < 0)
return err;
@@ -949,7 +949,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -961,7 +961,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, 2);
+ err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
if (err < 0)
return err;
@@ -975,7 +975,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -1409,7 +1409,7 @@ static void m88e1318_get_wol(struct phy_device *phydev,
MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
wol->wolopts |= WAKE_MAGIC;
- if (marvell_set_page(phydev, 0x00) < 0)
+ if (marvell_set_page(phydev, MII_M1111_COPPER) < 0)
return;
}
@@ -1422,7 +1422,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (wol->wolopts & WAKE_MAGIC) {
/* Explicitly switch to page 0x00, just to be sure */
- err = marvell_set_page(phydev, 0x00);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 net-next 0/4] More marvell phy cleanups
From: Andrew Lunn @ 2017-05-25 19:42 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
This patchset continues the cleanup of the Marvell PHY driver. These
phys use pages to allow more than the 32 registers that fit into the
MDIO address space. Cleanup the code used for changing pages.
v2
Reverse christmas tree
Andrew Lunn (4):
net: phy: marvell: #defines for copper and fibre pages
net: phy: marvell: More hidden page changes refactored
net: phy: marvell: helper to get and set page
net: phy: marvell: Uniform page names
drivers/net/phy/marvell.c | 181 +++++++++++++++++++++++++---------------------
1 file changed, 98 insertions(+), 83 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH v2 net-next 2/4] net: phy: marvell: More hidden page changes refactored
From: Andrew Lunn @ 2017-05-25 19:42 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495741328-25001-1-git-send-email-andrew@lunn.ch>
EXT_ADDR_PAGE is the same meaning as MII_MARVELL_PHY_PAGE, i.e. change
page. Replace it will calls to the helpers.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 62 +++++++++++++++++++++++++++++++++++------------
1 file changed, 46 insertions(+), 16 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index bb067026353a..3c577a177b2c 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -54,7 +54,6 @@
#define MII_M1011_PHY_SCR_MDI_X 0x0020
#define MII_M1011_PHY_SCR_AUTO_CROSS 0x0060
-#define MII_M1145_PHY_EXT_ADDR_PAGE 0x16
#define MII_M1145_PHY_EXT_SR 0x1b
#define MII_M1145_PHY_EXT_CR 0x14
#define MII_M1145_RGMII_RX_DELAY 0x0080
@@ -92,6 +91,7 @@
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(0x3 << 4))
+#define MII_88E1121_MISC_TEST_PAGE 6
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT 8
@@ -760,11 +760,7 @@ static int m88e1111_config_init_sgmii(struct phy_device *phydev)
return err;
/* make sure copper is selected */
- err = phy_read(phydev, MII_M1145_PHY_EXT_ADDR_PAGE);
- if (err < 0)
- return err;
-
- return phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, err & (~0xff));
+ return marvell_set_page(phydev, MII_M1111_COPPER);
}
static int m88e1111_config_init_rtbi(struct phy_device *phydev)
@@ -1554,6 +1550,7 @@ static void marvell_get_stats(struct phy_device *phydev,
#ifdef CONFIG_HWMON
static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
{
+ int oldpage;
int ret;
int val;
@@ -1561,7 +1558,13 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1593,7 +1596,7 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
*temp = ((val & MII_88E1121_MISC_TEST_TEMP_MASK) - 5) * 5000;
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1670,13 +1673,20 @@ static const struct hwmon_chip_info m88e1121_hwmon_chip_info = {
static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
{
+ int oldpage;
int ret;
*temp = 0;
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1687,7 +1697,7 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
*temp = ((ret & MII_88E1510_TEMP_SENSOR_MASK) - 25) * 1000;
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1695,13 +1705,19 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
{
+ int oldpage;
int ret;
*temp = 0;
mutex_lock(&phydev->lock);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1715,7 +1731,7 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
*temp *= 1000;
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1723,11 +1739,18 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
{
+ int oldpage;
int ret;
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1742,7 +1765,7 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
(temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT));
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1750,13 +1773,20 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
{
+ int oldpage;
int ret;
*alarm = false;
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1766,7 +1796,7 @@ int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
*alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 net-next 4/4] net: phy: marvell: Uniform page names
From: Andrew Lunn @ 2017-05-25 19:42 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495741328-25001-1-git-send-email-andrew@lunn.ch>
Bring all the page names together, remove the repeats, and make them
uniform.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 94 +++++++++++++++++++++++------------------------
1 file changed, 46 insertions(+), 48 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index ed338af61cdd..2bd83920f565 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -41,6 +41,12 @@
#include <linux/uaccess.h>
#define MII_MARVELL_PHY_PAGE 22
+#define MII_MARVELL_COPPER_PAGE 0x00
+#define MII_MARVELL_FIBER_PAGE 0x01
+#define MII_MARVELL_MSCR_PAGE 0x02
+#define MII_MARVELL_LED_PAGE 0x03
+#define MII_MARVELL_MISC_TEST_PAGE 0x06
+#define MII_MARVELL_WOL_PAGE 0x11
#define MII_M1011_IEVENT 0x13
#define MII_M1011_IEVENT_CLEAR 0x0000
@@ -82,16 +88,11 @@
#define MII_M1111_HWCFG_FIBER_COPPER_AUTO 0x8000
#define MII_M1111_HWCFG_FIBER_COPPER_RES 0x2000
-#define MII_M1111_COPPER 0
-#define MII_M1111_FIBER 1
-
-#define MII_88E1121_PHY_MSCR_PAGE 2
#define MII_88E1121_PHY_MSCR_REG 21
#define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5)
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(0x3 << 4))
-#define MII_88E1121_MISC_TEST_PAGE 6
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT 8
@@ -112,7 +113,6 @@
#define MII_88E1318S_PHY_CSIER_WOL_EIE BIT(7)
/* LED Timer Control Register */
-#define MII_88E1318S_PHY_LED_PAGE 0x03
#define MII_88E1318S_PHY_LED_TCR 0x12
#define MII_88E1318S_PHY_LED_TCR_FORCE_INT BIT(15)
#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE BIT(7)
@@ -123,13 +123,11 @@
#define MII_88E1318S_PHY_MAGIC_PACKET_WORD1 0x18
#define MII_88E1318S_PHY_MAGIC_PACKET_WORD0 0x19
-#define MII_88E1318S_PHY_WOL_PAGE 0x11
#define MII_88E1318S_PHY_WOL_CTRL 0x10
#define MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS BIT(12)
#define MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE BIT(14)
#define MII_88E1121_PHY_LED_CTRL 16
-#define MII_88E1121_PHY_LED_PAGE 3
#define MII_88E1121_PHY_LED_DEF 0x0030
#define MII_M1011_PHY_STATUS 0x11
@@ -465,7 +463,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (oldpage < 0)
return oldpage;
@@ -504,7 +502,7 @@ static int m88e1318_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (oldpage < 0)
return oldpage;
@@ -615,7 +613,7 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
{
int err;
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
@@ -625,7 +623,7 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
goto error;
/* Then the fiber link */
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -633,10 +631,10 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
if (err < 0)
goto error;
- return marvell_set_page(phydev, MII_M1111_COPPER);
+ return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -659,7 +657,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
mdelay(500);
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -671,7 +669,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (err < 0)
return err;
temp = phy_read(phydev, MII_M1116R_CONTROL_REG_MAC);
@@ -680,7 +678,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
err = phy_write(phydev, MII_M1116R_CONTROL_REG_MAC, temp);
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -769,7 +767,7 @@ static int m88e1111_config_init_sgmii(struct phy_device *phydev)
return err;
/* make sure copper is selected */
- return marvell_set_page(phydev, MII_M1111_COPPER);
+ return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
}
static int m88e1111_config_init_rtbi(struct phy_device *phydev)
@@ -852,7 +850,7 @@ static int m88e1121_config_init(struct phy_device *phydev)
{
int err, oldpage;
- oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_LED_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_LED_PAGE);
if (oldpage < 0)
return oldpage;
@@ -895,7 +893,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
return err;
/* Reset page selection */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
}
@@ -925,7 +923,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (err < 0)
return err;
@@ -935,7 +933,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Change address */
- err = marvell_set_page(phydev, MII_88E1318S_PHY_LED_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
if (err < 0)
return err;
@@ -952,7 +950,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -964,7 +962,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (err < 0)
return err;
@@ -978,7 +976,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -1248,7 +1246,7 @@ static int marvell_read_status_page(struct phy_device *phydev, int page)
/* Detect and update the link, but return if there
* was an error
*/
- if (page == MII_M1111_FIBER)
+ if (page == MII_MARVELL_FIBER_PAGE)
fiber = 1;
else
fiber = 0;
@@ -1281,11 +1279,11 @@ static int marvell_read_status(struct phy_device *phydev)
/* Check the fiber mode first */
if (phydev->supported & SUPPORTED_FIBRE &&
phydev->interface != PHY_INTERFACE_MODE_SGMII) {
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
- err = marvell_read_status_page(phydev, MII_M1111_FIBER);
+ err = marvell_read_status_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -1300,15 +1298,15 @@ static int marvell_read_status(struct phy_device *phydev)
return 0;
/* If fiber link is down, check and save copper mode state */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
}
- return marvell_read_status_page(phydev, MII_M1111_COPPER);
+ return marvell_read_status_page(phydev, MII_MARVELL_COPPER_PAGE);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -1323,7 +1321,7 @@ static int marvell_suspend(struct phy_device *phydev)
/* Suspend the fiber mode first */
if (!(phydev->supported & SUPPORTED_FIBRE)) {
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -1333,7 +1331,7 @@ static int marvell_suspend(struct phy_device *phydev)
goto error;
/* Then, the copper link */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
}
@@ -1342,7 +1340,7 @@ static int marvell_suspend(struct phy_device *phydev)
return genphy_suspend(phydev);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -1357,7 +1355,7 @@ static int marvell_resume(struct phy_device *phydev)
/* Resume the fiber mode first */
if (!(phydev->supported & SUPPORTED_FIBRE)) {
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -1367,7 +1365,7 @@ static int marvell_resume(struct phy_device *phydev)
goto error;
/* Then, the copper link */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
}
@@ -1376,7 +1374,7 @@ static int marvell_resume(struct phy_device *phydev)
return genphy_resume(phydev);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -1405,14 +1403,14 @@ static void m88e1318_get_wol(struct phy_device *phydev,
wol->supported = WAKE_MAGIC;
wol->wolopts = 0;
- if (marvell_set_page(phydev, MII_88E1318S_PHY_WOL_PAGE) < 0)
+ if (marvell_set_page(phydev, MII_MARVELL_WOL_PAGE) < 0)
return;
if (phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL) &
MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
wol->wolopts |= WAKE_MAGIC;
- if (marvell_set_page(phydev, MII_M1111_COPPER) < 0)
+ if (marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE) < 0)
return;
}
@@ -1425,7 +1423,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (wol->wolopts & WAKE_MAGIC) {
/* Explicitly switch to page 0x00, just to be sure */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -1436,7 +1434,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_88E1318S_PHY_LED_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
if (err < 0)
return err;
@@ -1449,7 +1447,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_88E1318S_PHY_WOL_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
return err;
@@ -1478,7 +1476,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (err < 0)
return err;
} else {
- err = marvell_set_page(phydev, MII_88E1318S_PHY_WOL_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
return err;
@@ -1564,7 +1562,7 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1682,7 +1680,7 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1710,7 +1708,7 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1739,7 +1737,7 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1771,7 +1769,7 @@ int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 net-next 3/4] net: phy: marvell: helper to get and set page
From: Andrew Lunn @ 2017-05-25 19:42 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495741328-25001-1-git-send-email-andrew@lunn.ch>
There is a common pattern of first reading the currently selected page
and then changing to another page. Add a helper to do this.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 75 ++++++++++++++++++++---------------------------
1 file changed, 31 insertions(+), 44 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 3c577a177b2c..ed338af61cdd 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -199,6 +199,19 @@ static int marvell_set_page(struct phy_device *phydev, int page)
return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
+static int marvell_get_set_page(struct phy_device *phydev, int page)
+{
+ int oldpage = marvell_get_page(phydev);
+
+ if (oldpage < 0)
+ return oldpage;
+
+ if (page != oldpage)
+ return marvell_set_page(phydev, page);
+
+ return 0;
+}
+
static int marvell_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -452,11 +465,9 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_page(phydev);
-
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
- if (err < 0)
- return err;
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ if (oldpage < 0)
+ return oldpage;
if (phy_interface_is_rgmii(phydev)) {
mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
@@ -493,11 +504,9 @@ static int m88e1318_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_page(phydev);
-
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
- if (err < 0)
- return err;
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ if (oldpage < 0)
+ return oldpage;
mscr = phy_read(phydev, MII_88E1318S_PHY_MSCR1_REG);
mscr |= MII_88E1318S_PHY_MSCR1_PAD_ODD;
@@ -843,11 +852,9 @@ static int m88e1121_config_init(struct phy_device *phydev)
{
int err, oldpage;
- oldpage = marvell_get_page(phydev);
-
- err = marvell_set_page(phydev, MII_88E1121_PHY_LED_PAGE);
- if (err < 0)
- return err;
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_LED_PAGE);
+ if (oldpage < 0)
+ return oldpage;
/* Default PHY LED config: LED[0] .. Link, LED[1] .. Activity */
err = phy_write(phydev, MII_88E1121_PHY_LED_CTRL,
@@ -1516,12 +1523,11 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
{
struct marvell_hw_stat stat = marvell_hw_stats[i];
struct marvell_priv *priv = phydev->priv;
- int err, oldpage, val;
+ int oldpage, val;
u64 ret;
- oldpage = marvell_get_page(phydev);
- err = marvell_set_page(phydev, stat.page);
- if (err < 0)
+ oldpage = marvell_get_set_page(phydev, stat.page);
+ if (oldpage < 0)
return UINT64_MAX;
val = phy_read(phydev, stat.reg);
@@ -1558,16 +1564,12 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
/* Enable temperature sensor */
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
@@ -1680,16 +1682,12 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1510_TEMP_SENSOR);
if (ret < 0)
goto error;
@@ -1711,16 +1709,13 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
*temp = 0;
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
@@ -1744,16 +1739,12 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
@@ -1780,16 +1771,12 @@ int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 6/6] net: mpls: minor cleanups
From: David Miller @ 2017-05-25 18:56 UTC (permalink / raw)
To: dsahern; +Cc: netdev, roopa
In-Reply-To: <20170525035442.51407-7-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Wed, 24 May 2017 21:54:42 -0600
> Noticed these doing the extack support:
> - nla_get_via is only used in af_mpls.c so remove from internal.h
...
> @@ -43,6 +43,9 @@ static void rtmsg_lfib(int event, u32 label, struct mpls_route *rt,
> struct nlmsghdr *nlh, struct net *net, u32 portid,
> unsigned int nlm_flags);
>
> +static int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
> + u8 via[], struct netlink_ext_ack *extack);
> +
> static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
> {
> struct mpls_route *rt = NULL;
David, please mark the actual defintion static as well.
I'm surprised the compiler accepts this, but it does :-)
^ permalink raw reply
* Re: [PATCH] bonding: Don't update slave->link until ready to commit
From: David Miller @ 2017-05-25 18:50 UTC (permalink / raw)
To: nsujir; +Cc: netdev, maheshb, jay.vosburgh
In-Reply-To: <1495680317-11308-1-git-send-email-nsujir@tintri.com>
From: Nithin Nayak Sujir <nsujir@tintri.com>
Date: Wed, 24 May 2017 19:45:17 -0700
> In the loadbalance arp monitoring scheme, when a slave link change is
> detected, the slave->link is immediately updated and slave_state_changed
> is set. Later down the function, the rtnl_lock is acquired and the
> changes are committed, updating the bond link state.
>
> However, the acquisition of the rtnl_lock can fail. The next time the
> monitor runs, since slave->link is already updated, it determines that
> link is unchanged. This results in the bond link state permanently out
> of sync with the slave link.
>
> This patch modifies bond_loadbalance_arp_mon() to handle link changes
> identical to bond_ab_arp_{inspect/commit}(). The new link state is
> maintained in slave->new_link until we're ready to commit at which point
> it's copied into slave->link.
>
> NOTE: miimon_{inspect/commit}() has a more complex state machine
> requiring the use of the bond_{propose,commit}_link_state() functions
> which maintains the intermediate state in slave->link_new_state. The arp
> monitors don't require that.
>
> Testing: This bug is very easy to reproduce with the following steps.
> 1. In a loop, toggle a slave link of a bond slave interface.
> 2. In a separate loop, do ifconfig up/down of an unrelated interface to
> create contention for rtnl_lock.
> Within a few iterations, the bond link goes out of sync with the slave
> link.
>
> Signed-off-by: Nithin Nayak Sujir <nsujir@tintri.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] ibmvnic: Enable TSO support
From: David Miller @ 2017-05-25 18:49 UTC (permalink / raw)
To: tlfalcon; +Cc: netdev, nfont, jallen
In-Reply-To: <20170525.144626.1787511445330790842.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Thu, 25 May 2017 14:46:26 -0400 (EDT)
> From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> Date: Wed, 24 May 2017 21:29:26 -0500
>
>> The feature is also enabled by a module parameter.
>> This parameter is necessary because TSO can not easily be
>> enabled or disabled in firmware without reinitializing the driver.
>
> Sorry, this is unacceptable. When I say no module parameters,
> I really really mean it.
>
> Users should not be burdoned with having to know a special knob for
> every driver in order to adjust what is a generic feature.
>
> You'll have to find another way to accomodate this.
Also, TSO helps without SG only because you haven't implemented
support for xmit_more in this driver to decrease the number of
doorball updates and VM enters.
I bet if you added xmit_more support, TSO wouldn't give you much
if any performance boost if you have to linearize.
^ permalink raw reply
* Re: [PATCH net-next] ibmvnic: Enable TSO support
From: David Miller @ 2017-05-25 18:46 UTC (permalink / raw)
To: tlfalcon; +Cc: netdev, nfont, jallen
In-Reply-To: <1495679366-7149-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Date: Wed, 24 May 2017 21:29:26 -0500
> The feature is also enabled by a module parameter.
> This parameter is necessary because TSO can not easily be
> enabled or disabled in firmware without reinitializing the driver.
Sorry, this is unacceptable. When I say no module parameters,
I really really mean it.
Users should not be burdoned with having to know a special knob for
every driver in order to adjust what is a generic feature.
You'll have to find another way to accomodate this.
^ permalink raw reply
* Re: [PATCH net-next 0/2] be2net: patch-set
From: David Miller @ 2017-05-25 18:45 UTC (permalink / raw)
To: suresh.reddy; +Cc: netdev
In-Reply-To: <1495679079-12025-1-git-send-email-suresh.reddy@broadcom.com>
From: Suresh Reddy <suresh.reddy@broadcom.com>
Date: Wed, 24 May 2017 22:24:37 -0400
> Hi Dave, Please consider applying these two patches to net-next
Series applied.
^ permalink raw reply
* Re: [PATCH] test_bpf: Add a couple of tests for BPF_JSGE.
From: David Miller @ 2017-05-25 18:39 UTC (permalink / raw)
To: david.daney; +Cc: ast, daniel, netdev, linux-kernel
In-Reply-To: <20170524233549.18388-1-david.daney@cavium.com>
From: David Daney <david.daney@cavium.com>
Date: Wed, 24 May 2017 16:35:49 -0700
> Some JITs can optimize comparisons with zero. Add a couple of
> BPF_JSGE tests against immediate zero.
>
> Signed-off-by: David Daney <david.daney@cavium.com>
Applied, thank you.
This reminds me that I should add support for "branch on register"
to the sparc64 eBPF JIT. At least now, when I do so, there will
be tests for it :-)
^ permalink raw reply
* Re: [PATCH net v2] sctp: fix ICMP processing if skb is non-linear
From: Vlad Yasevich @ 2017-05-25 17:58 UTC (permalink / raw)
To: Davide Caratti, netdev, linux-sctp
Cc: Xin Long, Marcelo Ricardo Leitner, David S . Miller
In-Reply-To: <d0cd7c2e4b9329ec23e48aed94bf2fbf94554f44.1495730857.git.dcaratti@redhat.com>
On 05/25/2017 01:14 PM, Davide Caratti wrote:
> sometimes ICMP replies to INIT chunks are ignored by the client, even if
> the encapsulated SCTP headers match an open socket. This happens when the
> ICMP packet is carried by a paged skb: use skb_header_pointer() to read
> packet contents beyond the SCTP header, so that chunk header and initiate
> tag are validated correctly.
>
> v2:
> - don't use skb_header_pointer() to read the transport header, since
> icmp_socket_deliver() already puts these 8 bytes in the linear area.
> - change commit message to make specific reference to INIT chunks.
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/input.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 0e06a27..ba9ad32 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -473,15 +473,14 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
> struct sctp_association **app,
> struct sctp_transport **tpp)
> {
> + struct sctp_init_chunk *chunkhdr, _chunkhdr;
> union sctp_addr saddr;
> union sctp_addr daddr;
> struct sctp_af *af;
> struct sock *sk = NULL;
> struct sctp_association *asoc;
> struct sctp_transport *transport = NULL;
> - struct sctp_init_chunk *chunkhdr;
> __u32 vtag = ntohl(sctphdr->vtag);
> - int len = skb->len - ((void *)sctphdr - (void *)skb->data);
>
> *app = NULL; *tpp = NULL;
>
> @@ -516,13 +515,16 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
> * discard the packet.
> */
> if (vtag == 0) {
> - chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
> - if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
> - + sizeof(__be32) ||
> + /* chunk header + first 4 octects of init header */
> + chunkhdr = skb_header_pointer(skb, skb_transport_offset(skb) +
> + sizeof(struct sctphdr),
> + sizeof(struct sctp_chunkhdr) +
> + sizeof(__be32), &_chunkhdr);
> + if (!chunkhdr ||
> chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
> - ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
> + ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
> goto out;
> - }
> +
> } else if (vtag != asoc->c.peer_vtag) {
> goto out;
> }
>
^ permalink raw reply
* Re: [PATCH net] sky2: Do not deadlock on sky2_hw_down
From: Stephen Hemminger @ 2017-05-25 17:54 UTC (permalink / raw)
To: Joshua Emele; +Cc: netdev, Joshua Emele, Mirko Lindner, linux-kernel
In-Reply-To: <20170524224353.124692-1-jemele@gmail.com>
On Wed, 24 May 2017 15:43:18 -0700
Joshua Emele <jemele@gmail.com> wrote:
> From: Joshua Emele <jemele@google.com>
>
> The sky2_hw_down uses sky2_tx_complete to free pending frames stuck in
> the HW queue. Because sky2_hw_down can be called from a process context,
> the call to u64_stats_update_begin can result in deadlock.
>
> Because the statistics do not require update as part of the sky2_hw_down
> sequence, prevent the update to avoid the deadlock.
>
> [18198.003900] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> [18198.009931] ifconfig/11604 [HC0[0]:SC0[0]:HE1:SE1] takes:
> [18198.015348] (&syncp->seq#2){+.?...}, at: [<805ed440>] sky2_hw_down+0x370/0x428
> [18198.022827] {IN-SOFTIRQ-W} state was registered at:
> [18198.027723] [<801729a0>] lock_acquire+0x78/0x98
> [18198.032484] [<805ed048>] sky2_tx_complete+0x1a8/0x230
> [18198.037760] [<805f2718>] sky2_poll+0x7cc/0xfa8
> [18198.042425] [<807692f8>] net_rx_action+0x200/0x330
> [18198.047447] [<80129f64>] __do_softirq+0x130/0x31c
> [18198.052369] [<8012a4a0>] irq_exit+0xc8/0x13c
> [18198.056836] [<8017e398>] __handle_domain_irq+0x84/0xf8
> [18198.062180] [<80101564>] gic_handle_irq+0x58/0xb8
> [18198.067086] [<8010d838>] __irq_usr+0x58/0x80
> [18198.071557] [<76e178ae>] 0x76e178ae
> [18198.075247] irq event stamp: 2109
> [18198.078568] hardirqs last enabled at (2109): [<8012a2cc>] __local_bh_enable_ip+0x90/0x110
> [18198.086860] hardirqs last disabled at (2107): [<8012a27c>] __local_bh_enable_ip+0x40/0x110
> [18198.095150] softirqs last enabled at (2108): [<805ed368>] sky2_hw_down+0x298/0x428
> [18198.102832] softirqs last disabled at (2106): [<805ed284>] sky2_hw_down+0x1b4/0x428
> [18198.110515]
> other info that might help us debug this:
> [18198.117048] Possible unsafe locking scenario:
>
> [18198.122974] CPU0
> [18198.125425] ----
> [18198.127876] lock(&syncp->seq#2);
> [18198.131332] <Interrupt>
> [18198.133955] lock(&syncp->seq#2);
> [18198.137585]
> *** DEADLOCK ***
>
> [18198.143517] 1 lock held by ifconfig/11604:
> [18198.147618] #0: (rtnl_mutex){+.+.+.}, at: [<8077e874>] rtnl_lock+0x18/0x20
> [18198.154772]
> stack backtrace:
> [18198.159143] CPU: 1 PID: 11604 Comm: ifconfig Not tainted 4.8.17 #1
> [18198.165331] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [18198.171864] Backtrace:
> [18198.174353] [<8010c634>] (dump_backtrace) from [<8010c828>] (show_stack+0x18/0x1c)
> [18198.181931] r6:60070093 r5:00000000 r4:80e23248 r3:00000000
> [18198.187682] [<8010c810>] (show_stack) from [<8041edc4>] (dump_stack+0xb4/0xe8)
> [18198.194924] [<8041ed10>] (dump_stack) from [<801700cc>] (print_usage_bug+0x1dc/0x2d0)
> [18198.202762] r10:edec55c0 r9:80bd1178 r8:81649598 r7:00000006 r6:edec5a90 r5:80fb09f4
> [18198.210686] r4:edec55c0 r3:00000002
> [18198.214311] [<8016fef0>] (print_usage_bug) from [<80170348>] (mark_lock+0x188/0x6c4)
> [18198.222059] r9:00000000 r8:00000004 r7:edec55c0 r6:00000006 r5:edec5a90 r4:8016f44c
> [18198.229903] [<801701c0>] (mark_lock) from [<801714ac>] (__lock_acquire+0xb90/0x1c70)
> [18198.237653] r10:edec55c0 r9:00000000 r8:edec5aa4 r7:00000004 r6:00000001 r5:00000000
> [18198.245573] r4:000001cd r3:00000001
> [18198.249192] [<8017091c>] (__lock_acquire) from [<801729a0>] (lock_acquire+0x78/0x98)
> [18198.256940] r10:00000001 r9:000006d0 r8:00000000 r7:00000001 r6:805ed440 r5:60070013
> [18198.264860] r4:00000000
> [18198.267428] [<80172928>] (lock_acquire) from [<805ed048>] (sky2_tx_complete+0x1a8/0x230)
> [18198.275523] r7:ee2d9dfc r6:00000000 r5:ee2d9dc0 r4:00000000
> [18198.281266] [<805ecea0>] (sky2_tx_complete) from [<805ed440>] (sky2_hw_down+0x370/0x428)
> [18198.289360] r10:ee2cac00 r9:000006d0 r8:ee2d9dc0 r7:f0d40aa8 r6:00000001 r5:00000d48
> [18198.297281] r4:00000000
> [18198.299850] [<805ed0d0>] (sky2_hw_down) from [<805efed8>] (sky2_close+0xac/0x11c)
> [18198.307337] r10:ee838600 r9:00000000 r8:00000000 r7:00001003 r6:ee2d9dc0 r5:00000000
> [18198.315258] r4:ee2cac00
> [18198.317830] [<805efe2c>] (sky2_close) from [<8076a310>] (__dev_close_many+0xc4/0x118)
> [18198.325664] r7:00001003 r6:00001042 r5:eb047df8 r4:ee2d9800
> [18198.331404] [<8076a24c>] (__dev_close_many) from [<8076ab24>] (__dev_close+0x30/0x48)
> [18198.339240] r5:00000001 r4:ee2d9800
> [18198.342866] [<8076aaf4>] (__dev_close) from [<8076dbac>] (__dev_change_flags+0x90/0x154)
> [18198.350970] [<8076db1c>] (__dev_change_flags) from [<8076dc90>] (dev_change_flags+0x20/0x50)
> [18198.359412] r8:00000000 r7:00008914 r6:00001003 r5:ee2d9948 r4:ee2d9800 r3:00000014
> [18198.367264] [<8076dc70>] (dev_change_flags) from [<807f06d0>] (devinet_ioctl+0x724/0x818)
> [18198.375448] r8:ffffffff r7:00008914 r6:ee2cae0c r5:7ed799ac r4:eb047e80 r3:00000014
> [18198.383291] [<807effac>] (devinet_ioctl) from [<807f2e98>] (inet_ioctl+0x19c/0x1c8)
> [18198.390951] r10:edfc3b80 r9:eb046000 r8:00000004 r7:ee724b40 r6:7ed799ac r5:ee724b60
> [18198.398872] r4:00008914
> [18198.401446] [<807f2cfc>] (inet_ioctl) from [<8074a234>] (sock_ioctl+0x158/0x300)
> [18198.408860] [<8074a0dc>] (sock_ioctl) from [<80239770>] (do_vfs_ioctl+0x98/0xa3c)
> [18198.416348] r7:8023a150 r6:00000004 r5:ee724b60 r4:7ed799ac
> [18198.422087] [<802396d8>] (do_vfs_ioctl) from [<8023a150>] (SyS_ioctl+0x3c/0x64)
> [18198.429401] r10:00000000 r9:eb046000 r8:00000004 r7:00008914 r6:edfc3b80 r5:7ed799ac
> [18198.437326] r4:edfc3b80
> [18198.439895] [<8023a114>] (SyS_ioctl) from [<80107ee0>] (ret_fast_syscall+0x0/0x1c)
> [18198.447469] r8:80108084 r7:00000036 r6:00000004 r5:7ed799ac r4:7ed79b20 r3:31687465
>
> Signed-off-by: Joshua Emele <jemele@google.com>
This problem was introduced by the "better version of seqcount". The original
version which is the raw_XXX version does not have any locking.
Sigh. The point of u64_stats_update_begin/end was that they are supposed to be lock less.
What architecture are you using that has such a broken version which uses locks?
^ permalink raw reply
* [PATCH net-next] liquidio: fix inaccurate count of napi-processed rx packets reported to Octeon
From: Felix Manlunas @ 2017-05-25 17:54 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
prasad.kanneganti
From: Prasad Kanneganti <prasad.kanneganti@cavium.com>
lio_enable_irq (called by napi poll) is reporting to Octeon an inaccurate
count of processed rx packets causing Octeon to eventually stop forwarding
packets to the host. Fix it by using this formula for an accurate count:
processed rx packets = droq->pkt_count - droq->pkts_pending
Also increase SOFT_COMMAND_BUFFER_SIZE to match what the firmware expects.
Signed-off-by: Prasad Kanneganti <prasad.kanneganti@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/octeon_device.c | 6 ++++--
drivers/net/ethernet/cavium/liquidio/octeon_iq.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index b5be707..3b7cc93 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -1429,13 +1429,15 @@ int lio_get_device_id(void *dev)
void lio_enable_irq(struct octeon_droq *droq, struct octeon_instr_queue *iq)
{
u64 instr_cnt;
+ u32 pkts_pend;
struct octeon_device *oct = NULL;
/* the whole thing needs to be atomic, ideally */
if (droq) {
+ pkts_pend = (u32)atomic_read(&droq->pkts_pending);
spin_lock_bh(&droq->lock);
- writel(droq->pkt_count, droq->pkts_sent_reg);
- droq->pkt_count = 0;
+ writel(droq->pkt_count - pkts_pend, droq->pkts_sent_reg);
+ droq->pkt_count = pkts_pend;
/* this write needs to be flushed before we release the lock */
mmiowb();
spin_unlock_bh(&droq->lock);
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_iq.h b/drivers/net/ethernet/cavium/liquidio/octeon_iq.h
index 5063a12..5c3c8da 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_iq.h
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_iq.h
@@ -251,7 +251,7 @@ union octeon_instr_64B {
/** The size of each buffer in soft command buffer pool
*/
-#define SOFT_COMMAND_BUFFER_SIZE 1536
+#define SOFT_COMMAND_BUFFER_SIZE 2048
struct octeon_soft_command {
/** Soft command buffer info. */
^ permalink raw reply related
* RE: [PATCH net-next] liquidio: fix rare pci_driver.probe failure of VF driver
From: Chickles, Derek @ 2017-05-25 17:52 UTC (permalink / raw)
To: Manlunas, Felix, davem@davemloft.net
Cc: netdev@vger.kernel.org, Vatsavayi, Raghu, Burla, Satananda,
Kanneganti, Prasad
In-Reply-To: <20170525174214.GA1444@felix-thinkpad.cavium.com>
Looks good to me.
Derek
> -----Original Message-----
> From: Manlunas, Felix
> Sent: Thursday, May 25, 2017 10:42 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; Vatsavayi, Raghu <Raghu.Vatsavayi@cavium.com>;
> Chickles, Derek <Derek.Chickles@cavium.com>; Burla, Satananda
> <Satananda.Burla@cavium.com>; Kanneganti, Prasad
> <Prasad.Kanneganti@cavium.com>
> Subject: [PATCH net-next] liquidio: fix rare pci_driver.probe failure of
> VF driver
>
> From: Prasad Kanneganti <prasad.kanneganti@cavium.com>
>
> There's a rare pci_driver.probe failure of the VF driver that's caused by
> PF/VF handshake going out of sync. The culprit is octeon_mbox_write() who
> ignores an ack timeout condition; it just keeps unconditionally writing
> all elements of mbox_cmd->data[] even when the other side is not ready for
> them. Fix it by making each write of mbox_cmd->data[i] conditional to
> having previously received an ack.
>
> Also fix the octeon_mbox_state enum such that each state gets a unique
> value. Also add ULL suffix to numeric literals in macro definitions.
>
> Signed-off-by: Prasad Kanneganti <prasad.kanneganti@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> ---
> drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c | 5 ++++-
> drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h | 12 ++++++------
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> index 5cca73b..57af7df 100644
> --- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> +++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> @@ -178,7 +178,10 @@ int octeon_mbox_write(struct octeon_device *oct,
> break;
> }
> }
> - writeq(mbox_cmd->data[i], mbox->mbox_write_reg);
> + if (ret == OCTEON_MBOX_STATUS_SUCCESS)
> + writeq(mbox_cmd->data[i], mbox->mbox_write_reg);
> + else
> + break;
> }
> }
>
> diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> index c9376fe..1def22a 100644
> --- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> +++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> @@ -20,16 +20,16 @@
>
> /* Macros for Mail Box Communication */
>
> -#define OCTEON_MBOX_DATA_MAX 32
> +#define OCTEON_MBOX_DATA_MAX 32
>
> #define OCTEON_VF_ACTIVE 0x1
> #define OCTEON_VF_FLR_REQUEST 0x2
> #define OCTEON_PF_CHANGED_VF_MACADDR 0x4
>
> /*Macro for Read acknowldgement*/
> -#define OCTEON_PFVFACK 0xffffffffffffffff
> -#define OCTEON_PFVFSIG 0x1122334455667788
> -#define OCTEON_PFVFERR 0xDEADDEADDEADDEAD
> +#define OCTEON_PFVFACK 0xffffffffffffffffULL
> +#define OCTEON_PFVFSIG 0x1122334455667788ULL
> +#define OCTEON_PFVFERR 0xDEADDEADDEADDEADULL
>
> #define LIO_MBOX_WRITE_WAIT_CNT 1000
> #define LIO_MBOX_WRITE_WAIT_TIME msecs_to_jiffies(1)
> @@ -74,8 +74,8 @@ enum octeon_mbox_state {
> OCTEON_MBOX_STATE_REQUEST_RECEIVED = 4,
> OCTEON_MBOX_STATE_RESPONSE_PENDING = 8,
> OCTEON_MBOX_STATE_RESPONSE_RECEIVING = 16,
> - OCTEON_MBOX_STATE_RESPONSE_RECEIVED = 16,
> - OCTEON_MBOX_STATE_ERROR = 32
> + OCTEON_MBOX_STATE_RESPONSE_RECEIVED = 32,
> + OCTEON_MBOX_STATE_ERROR = 64
> };
>
> struct octeon_mbox {
^ 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