* Re: [PATCH 0/3] netfilter fixes for your net tree
From: David Miller @ 2013-03-07 20:23 UTC (permalink / raw)
To: pablo; +Cc: netdev, netfilter-devel
In-Reply-To: <1362654002-4405-1-git-send-email-pablo@netfilter.org>
From: pablo@netfilter.org
Date: Thu, 7 Mar 2013 11:59:59 +0100
> The following patchset contains Netfilter fixes for your net tree,
> they are:
>
> * Don't generate audit log message if audit is not enabled, from Gao Feng.
>
> * Fix logging formatting for packets dropped by helpers, by Joe Perches.
>
> * Fix a compilation warning in nfnetlink if CONFIG_PROVE_RCU is not set,
> from Paul Bolle.
>
> You can pull these changes from:
>
> git://1984.lsi.us.es/nf master
Pulled, but keep in mind that if there are any bugs in these changes I
know exactly where you are. :-)
^ permalink raw reply
* [PATCH] macvlan: Set IFF_UNICAST_FLT flag to prevent unnecessary promisc mode.
From: Vlad Yasevich @ 2013-03-07 20:21 UTC (permalink / raw)
To: netdev; +Cc: Vlad Yasevich
Macvlan already supports hw address filters. Set the IFF_UNICAST_FLT
so that it doesn't needlesly enter PROMISC mode when macvlans are
stacked.
Signed-of-by: Vlad Yasevich <vyasevic@redhat.com>
---
drivers/net/macvlan.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 417b2af..73abbc1 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -660,6 +660,7 @@ void macvlan_common_setup(struct net_device *dev)
ether_setup(dev);
dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
+ dev->priv_flags |= IFF_UNICAST_FLT;
dev->netdev_ops = &macvlan_netdev_ops;
dev->destructor = free_netdev;
dev->header_ops = &macvlan_hard_header_ops,
--
1.7.7.6
^ permalink raw reply related
* Re: [net 0/3][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2013-03-07 20:19 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann, bruce.w.allan
In-Reply-To: <1362654925-26088-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 7 Mar 2013 03:15:22 -0800
> This series contains updates to e1000e only.
>
> All three patches come from Konstantin Khlebnikov to resolve power
> management issues. The first patch removes redundant and unbalanced
> pci_disable_device() from the shutdown function. The second patch
> removes redundant actions from the driver and fixes the interaction
> with actions in pci-bus runtime power management code. The third
> and last patch fixes some messages like 'Error reading PHY register'
> and 'Hardware Erorr' and saves several seconds on reboot.
>
> The following are changes since commit f8af75f3517a24838a36eb5797a1a3e60bf9e276:
> tun: add a missing nf_reset() in tun_net_xmit()
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master
Pulled, thanks Jeff.
^ permalink raw reply
* [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices
From: Alexander Aring @ 2013-03-07 20:13 UTC (permalink / raw)
To: davem
Cc: kuznet, jmorris, yoshfuji, linux-zigbee-devel, netdev,
Alexander Aring
The function addrconf_ifid_eui64 will copy eui into dev->dev_addr.
We need first to manipulate eui[0] before we call memcpy afterwards.
Broken since commit:
5e98a36ed4bf6ea396170e3af0dd4fcbe51d772f
Since this commit I got many trouble with the ieee802154 stack.
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
net/ipv6/addrconf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index fa36a67..4460237 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1731,8 +1731,8 @@ static int addrconf_ifid_eui64(u8 *eui, struct net_device *dev)
{
if (dev->addr_len != IEEE802154_ADDR_LEN)
return -1;
- memcpy(eui, dev->dev_addr, 8);
eui[0] ^= 2;
+ memcpy(eui, dev->dev_addr, 8);
return 0;
}
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
From: Ben Hutchings @ 2013-03-07 19:59 UTC (permalink / raw)
To: Peter Korsgaard
Cc: Mugunthan V N, netdev, davem, devicetree-discuss, linux-omap,
b-cousson, paul
In-Reply-To: <8762135on4.fsf@dell.be.48ers.dk>
On Thu, 2013-03-07 at 14:24 +0100, Peter Korsgaard wrote:
> >>>>> "M" == Mugunthan V N <mugunthanvnm@ti.com> writes:
>
> M> This patch implements get/set of the phy settings via ethtool apis
> M> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> M> ---
> M> Documentation/devicetree/bindings/net/cpsw.txt | 3 +++
> M> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++++++
> M> include/linux/platform_data/cpsw.h | 1 +
> M> 3 files changed, 36 insertions(+)
>
> M> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> M> index ecfdf75..8d61300 100644
> M> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> M> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> M> @@ -20,6 +20,7 @@ Required properties:
> M> - cpts_clock_shift : Denominator to convert input clock ticks into nanoseconds
> M> - phy_id : Specifies slave phy id
> M> - mac-address : Specifies slave MAC address
> M> +- ethtool-active-slave : Specifies the slave to use for ethtool command
>
> That again sounds like something Linux specific rather than a hardware
> property.
Yes, indeed. Isn't it redundant with the phy_id?
Ben.
> It would be good if all these special things (dual emac mode, vlan
> handling, switching) could be handled using the existing kernel
> (bridging/vlan) infrastructure, and the driver always just exposing 2
> network interfaces instead of these configuration properties.
>
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH net-next 2/2] tunnel: Inherit NETIF_F_SG for hw_enc_features.
From: Pravin B Shelar @ 2013-03-07 19:28 UTC (permalink / raw)
To: netdev; +Cc: edumazet, jesse, Pravin B Shelar
Inherit scatergather feature for tunnel devices to avoid
copy for TSO packets of tunneling device like GRE.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
net/core/dev.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index bb99993..90cee5b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5205,6 +5205,10 @@ int register_netdevice(struct net_device *dev)
*/
dev->vlan_features |= NETIF_F_HIGHDMA;
+ /* Make NETIF_F_SG inheritable to tunnel devices.
+ */
+ dev->hw_enc_features |= NETIF_F_SG;
+
ret = call_netdevice_notifiers(NETDEV_POST_INIT, dev);
ret = notifier_to_errno(ret);
if (ret)
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 1/2] net: Kill link between CSUM and SG features.
From: Pravin B Shelar @ 2013-03-07 19:28 UTC (permalink / raw)
To: netdev; +Cc: edumazet, jesse, Pravin B Shelar
Earlier SG was unset if CSUM was not available for given device to
force skb copy to avoid sending inconsistent csum.
Commit c9af6db4c11c (net: Fix possible wrong checksum generation)
added explicit flag to force copy to fix this issue. Therefore
there is no need to link SG and CSUM, following patch kills this
link between there two features.
This patch is also required following patch in series.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
include/linux/netdevice.h | 13 +++++++++
net/core/dev.c | 63 +++++++++++++++++++++++---------------------
net/core/skbuff.c | 13 +++++++++
net/ipv4/af_inet.c | 3 --
net/ipv6/ip6_offload.c | 3 --
5 files changed, 59 insertions(+), 36 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 896eb49..e1ebeff 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2683,6 +2683,19 @@ struct sk_buff *skb_gso_segment(struct sk_buff *skb, netdev_features_t features)
{
return __skb_gso_segment(skb, features, true);
}
+__be16 skb_network_protocol(struct sk_buff *skb);
+
+static inline bool can_checksum_protocol(netdev_features_t features,
+ __be16 protocol)
+{
+ return ((features & NETIF_F_GEN_CSUM) ||
+ ((features & NETIF_F_V4_CSUM) &&
+ protocol == htons(ETH_P_IP)) ||
+ ((features & NETIF_F_V6_CSUM) &&
+ protocol == htons(ETH_P_IPV6)) ||
+ ((features & NETIF_F_FCOE_CRC) &&
+ protocol == htons(ETH_P_FCOE)));
+}
#ifdef CONFIG_BUG
extern void netdev_rx_csum_fault(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 9610389..bb99993 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2208,16 +2208,8 @@ out:
}
EXPORT_SYMBOL(skb_checksum_help);
-/**
- * skb_mac_gso_segment - mac layer segmentation handler.
- * @skb: buffer to segment
- * @features: features for the output path (see dev->features)
- */
-struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
- netdev_features_t features)
+__be16 skb_network_protocol(struct sk_buff *skb)
{
- struct sk_buff *segs = ERR_PTR(-EPROTONOSUPPORT);
- struct packet_offload *ptype;
__be16 type = skb->protocol;
while (type == htons(ETH_P_8021Q)) {
@@ -2225,13 +2217,31 @@ struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
struct vlan_hdr *vh;
if (unlikely(!pskb_may_pull(skb, vlan_depth + VLAN_HLEN)))
- return ERR_PTR(-EINVAL);
+ return 0;
vh = (struct vlan_hdr *)(skb->data + vlan_depth);
type = vh->h_vlan_encapsulated_proto;
vlan_depth += VLAN_HLEN;
}
+ return type;
+}
+
+/**
+ * skb_mac_gso_segment - mac layer segmentation handler.
+ * @skb: buffer to segment
+ * @features: features for the output path (see dev->features)
+ */
+struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
+ netdev_features_t features)
+{
+ struct sk_buff *segs = ERR_PTR(-EPROTONOSUPPORT);
+ struct packet_offload *ptype;
+ __be16 type = skb_network_protocol(skb);
+
+ if (unlikely(!type))
+ return ERR_PTR(-EINVAL);
+
__skb_pull(skb, skb->mac_len);
rcu_read_lock();
@@ -2398,24 +2408,12 @@ static int dev_gso_segment(struct sk_buff *skb, netdev_features_t features)
return 0;
}
-static bool can_checksum_protocol(netdev_features_t features, __be16 protocol)
-{
- return ((features & NETIF_F_GEN_CSUM) ||
- ((features & NETIF_F_V4_CSUM) &&
- protocol == htons(ETH_P_IP)) ||
- ((features & NETIF_F_V6_CSUM) &&
- protocol == htons(ETH_P_IPV6)) ||
- ((features & NETIF_F_FCOE_CRC) &&
- protocol == htons(ETH_P_FCOE)));
-}
-
static netdev_features_t harmonize_features(struct sk_buff *skb,
__be16 protocol, netdev_features_t features)
{
if (skb->ip_summed != CHECKSUM_NONE &&
!can_checksum_protocol(features, protocol)) {
features &= ~NETIF_F_ALL_CSUM;
- features &= ~NETIF_F_SG;
} else if (illegal_highdma(skb->dev, skb)) {
features &= ~NETIF_F_SG;
}
@@ -4921,20 +4919,25 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
}
- /* Fix illegal SG+CSUM combinations. */
- if ((features & NETIF_F_SG) &&
- !(features & NETIF_F_ALL_CSUM)) {
- netdev_dbg(dev,
- "Dropping NETIF_F_SG since no checksum feature.\n");
- features &= ~NETIF_F_SG;
- }
-
/* TSO requires that SG is present as well. */
if ((features & NETIF_F_ALL_TSO) && !(features & NETIF_F_SG)) {
netdev_dbg(dev, "Dropping TSO features since no SG feature.\n");
features &= ~NETIF_F_ALL_TSO;
}
+ if ((features & NETIF_F_TSO) && !(features & NETIF_F_HW_CSUM) &&
+ !(features & NETIF_F_IP_CSUM)) {
+ netdev_dbg(dev, "Dropping TSO features since no CSUM feature.\n");
+ features &= ~NETIF_F_TSO;
+ features &= ~NETIF_F_TSO_ECN;
+ }
+
+ if ((features & NETIF_F_TSO6) && !(features & NETIF_F_HW_CSUM) &&
+ !(features & NETIF_F_IPV6_CSUM)) {
+ netdev_dbg(dev, "Dropping TSO6 features since no CSUM feature.\n");
+ features &= ~NETIF_F_TSO6;
+ }
+
/* TSO ECN requires that TSO is present as well. */
if ((features & NETIF_F_ALL_TSO) == NETIF_F_TSO_ECN)
features &= ~NETIF_F_TSO_ECN;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 33245ef..0a48ae2 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2741,12 +2741,19 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
unsigned int tnl_hlen = skb_tnl_header_len(skb);
unsigned int headroom;
unsigned int len;
+ __be16 proto;
+ bool csum;
int sg = !!(features & NETIF_F_SG);
int nfrags = skb_shinfo(skb)->nr_frags;
int err = -ENOMEM;
int i = 0;
int pos;
+ proto = skb_network_protocol(skb);
+ if (unlikely(!proto))
+ return ERR_PTR(-EINVAL);
+
+ csum = !!can_checksum_protocol(features, proto);
__skb_push(skb, doffset);
headroom = skb_headroom(skb);
pos = skb_headlen(skb);
@@ -2884,6 +2891,12 @@ skip_fraglist:
nskb->data_len = len - hsize;
nskb->len += nskb->data_len;
nskb->truesize += nskb->data_len;
+
+ if (!csum) {
+ nskb->csum = skb_checksum(nskb, doffset,
+ nskb->len - doffset, 0);
+ nskb->ip_summed = CHECKSUM_NONE;
+ }
} while ((offset += len) < skb->len);
return segs;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 68f6a94..dc3f677 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1284,9 +1284,6 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
int id;
unsigned int offset = 0;
- if (!(features & NETIF_F_V4_CSUM))
- features &= ~NETIF_F_SG;
-
if (unlikely(skb_shinfo(skb)->gso_type &
~(SKB_GSO_TCPV4 |
SKB_GSO_UDP |
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 8234c1d..7a0d25a 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -92,9 +92,6 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
u8 *prevhdr;
int offset = 0;
- if (!(features & NETIF_F_V6_CSUM))
- features &= ~NETIF_F_SG;
-
if (unlikely(skb_shinfo(skb)->gso_type &
~(SKB_GSO_UDP |
SKB_GSO_DODGY |
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] 8139too: send NETDEV_CHANGE manually when autoneg is disabled
From: Ben Hutchings @ 2013-03-07 19:10 UTC (permalink / raw)
To: Veaceslav Falico
Cc: David Miller, netdev, wfp5p, jasowang, junchangwang, greearb,
ivecera
In-Reply-To: <20130307183803.GB21810@redhat.com>
On Thu, 2013-03-07 at 19:38 +0100, Veaceslav Falico wrote:
> On Thu, Mar 07, 2013 at 04:54:21PM +0000, Ben Hutchings wrote:
> >On Thu, 2013-03-07 at 17:35 +0100, Veaceslav Falico wrote:
> >> On Thu, Mar 07, 2013 at 03:52:35PM +0000, Ben Hutchings wrote:
> >> >On Thu, 2013-03-07 at 11:27 +0100, Veaceslav Falico wrote:
[...]
> >> >> Silently changing speed can break things a bit - bonding relies on
> >> >> interface speeds for 802.3ad/alb/tlb/active-backup iirc, bridge relies on
> >> >> stp port cost etc. and they all get it via NETDEV_ notifications. So
> >> >> without them, they would end up with outdated data, per example (eth2 being
> >> >> 8139too):
> >> >[...]
> >> >
> >> >Yes, I get it. But on real hardware, changing speed/duplex is always
> >> >going to break the link if it's up. The link change notification should
> >> >result in kernel and userland notifications via linkwatch_do_dev().
> >> >
> >> >(If you're testing against QEMU rather than real hardware, there could
> >> >be a bug in the emulation of link interrupts.)
> >>
> >> It's real hardware:
> >>
> >> [ 4.339413] 8139cp 0000:10:09.0: This (id 10ec:8139 rev 10) is not an 8139C+ compatible chip, use 8139too
> >> [ 4.348210] 8139too: 8139too Fast Ethernet driver 0.9.28
> >> [ 4.350017] 8139too 0000:10:09.0 eth2: RealTek RTL8139 at 0xffffc90004574100, 00:14:d1:1f:b9:49, IRQ 21
> >[...]
> >
> >OK. But it's generally not enough to send a 'something changed'
> >notification; the driver actually has to update the link state. MAybe
> >in your test you reconfigure the other end of the link too, or it's
> >smart enough to do parallel detect. But in general, changing the link
> >speed is going to change the link state, and the TX scheduler needs to
> >enabled or disabled accordingly.
>
> Sorry, I think I didn't explain it clearly. The driver *does* notify
> about link changes even when it has autonegotiation disabled.
>
> The only thing that doesn't work is the notification when someone changes
> it via ethtool. In the same moment.
The link *should* go down (assuming it was up before) because you likely
have a speed mismatch and won't be able to pass traffic. I don't know
whether this is possible for the hardware to detect immediately; maybe a
PHY reset will ensure that it is detected.
[...]
> Other drivers, after setting autoneg off, usually ifdown/ifup the interface
> manually, and thus everything gets updated. That is another way to fix this
> driver - to issue ->ndo_stop(); ->ndo_open(); (as netxen_nic/qlcnic/etc
> do), but I think that just issuing a notification that we've changed the
> state, without flapping, is better.
The link *should* flap, though doing a full stop/start is likely
unnecessary.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
From: Peter Korsgaard @ 2013-03-07 18:49 UTC (permalink / raw)
To: Mugunthan V N
Cc: netdev, davem, devicetree-discuss, linux-omap, b-cousson, paul
In-Reply-To: <5138CAA8.1060106@ti.com>
>>>>> "Mugunthan" == Mugunthan V N <mugunthanvnm@ti.com> writes:
Hi,
M> +- ethtool-active-slave : Specifies the slave to use for ethtool command
>>
>> That again sounds like something Linux specific rather than a hardware
>> property.
>>
>> It would be good if all these special things (dual emac mode, vlan
>> handling, switching) could be handled using the existing kernel
>> (bridging/vlan) infrastructure, and the driver always just exposing 2
>> network interfaces instead of these configuration properties.
Mugunthan> Switch and Dual Emac modes of operation of CPSW are two
Mugunthan> different features of the hardware and packet routing
Mugunthan> between the slaves in the hardware are different in both the
Mugunthan> modes.
Mugunthan> If by default it is brought up as Dual EMAC then hardware
Mugunthan> switching is blocked and use-cases like IP phone etc cannot
Mugunthan> be achieved.
Well, you could use the (sw) bridge functionality of the kernel network
stack, but performance naturally wouldn't be as good.
Mugunthan> Since CPSW as a hardware Switch, it cannot not be handled in
Mugunthan> existing kernel feature.
Well, we do have net/dsa, which is conceptually quite similar (even
though it has never been extended to hook into the bridging stuff). I
agree that we don't have infrastructure to handle hw like cpsw in a
really good way today, but it would be very nice to move towards it.
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH] 8139too: send NETDEV_CHANGE manually when autoneg is disabled
From: Veaceslav Falico @ 2013-03-07 18:38 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Miller, netdev, wfp5p, jasowang, junchangwang, greearb,
ivecera
In-Reply-To: <1362675261.2936.18.camel@bwh-desktop.uk.solarflarecom.com>
On Thu, Mar 07, 2013 at 04:54:21PM +0000, Ben Hutchings wrote:
>On Thu, 2013-03-07 at 17:35 +0100, Veaceslav Falico wrote:
>> On Thu, Mar 07, 2013 at 03:52:35PM +0000, Ben Hutchings wrote:
>> >On Thu, 2013-03-07 at 11:27 +0100, Veaceslav Falico wrote:
>> >> On Wed, Mar 06, 2013 at 03:53:23PM -0500, David Miller wrote:
>> >> >From: Ben Hutchings <bhutchings@solarflare.com>
>> >> >Date: Wed, 6 Mar 2013 20:22:52 +0000
>> >> >
>> >> >> On Wed, 2013-03-06 at 20:06 +0100, Veaceslav Falico wrote:
>> >> >>> When setting autoneg off (with any additional parameters, like
>> >> >>> speed/duplex), 8139too doesn't do an interface reset, and thus doesn't
>> >> >>> notify anyone that its speed/duplex might have changed (bonding and bridge
>> >> >>> will not see the speed changes, per example).
>> >> >>>
>> >> >>> Verify if we've force_media and send notification manually, so that the
>> >> >>> listeners have a chance to see the changes. It's quite ugly, however I
>> >> >>> don't see anything better.
>> >> >>
>> >> >> Isn't this really a bug in mii_check_media()? It shouldn't shortcut the
>> >> >> calls to netif_carrier_{off,on}() just because mii->force_media is set.
>> >> >
>> >> >I think mii_check_media() is responsible for handling this too.
>> >>
>> >> The mii_check_media() doesn't get called, AFAIK. The problem here is that,
>> >> after we call ethtool -s eth0 autoneg off speed X, with eth0 being
>> >> 8139too, the speed/autoneg options are changed via mii_ethtool_sset(),
>> >> however the interface itself isn't down'ed/up'ed, and thus no NETDEV_
>> >> notifications are sent.
>> >
>> >Does the hardware not send link interrupts if autoneg is disabled?
>>
>> Yes, it doesn't send them when the autoneg off is set.
>>
>> >
>> >> Other drivers either explicitly reset the interface after
>> >> ethtool.set_settings() call (like netxen_nic using ndo_close()/ndo_open()),
>> >> do it on the logic level (tg3) without involving mii_ethtool_sset(), or
>> >> just reset on their own (e100 iirc), so that most of them are responsible
>> >> for somehow triggering these events.
>> >>
>> >> Silently changing speed can break things a bit - bonding relies on
>> >> interface speeds for 802.3ad/alb/tlb/active-backup iirc, bridge relies on
>> >> stp port cost etc. and they all get it via NETDEV_ notifications. So
>> >> without them, they would end up with outdated data, per example (eth2 being
>> >> 8139too):
>> >[...]
>> >
>> >Yes, I get it. But on real hardware, changing speed/duplex is always
>> >going to break the link if it's up. The link change notification should
>> >result in kernel and userland notifications via linkwatch_do_dev().
>> >
>> >(If you're testing against QEMU rather than real hardware, there could
>> >be a bug in the emulation of link interrupts.)
>>
>> It's real hardware:
>>
>> [ 4.339413] 8139cp 0000:10:09.0: This (id 10ec:8139 rev 10) is not an 8139C+ compatible chip, use 8139too
>> [ 4.348210] 8139too: 8139too Fast Ethernet driver 0.9.28
>> [ 4.350017] 8139too 0000:10:09.0 eth2: RealTek RTL8139 at 0xffffc90004574100, 00:14:d1:1f:b9:49, IRQ 21
>[...]
>
>OK. But it's generally not enough to send a 'something changed'
>notification; the driver actually has to update the link state. MAybe
>in your test you reconfigure the other end of the link too, or it's
>smart enough to do parallel detect. But in general, changing the link
>speed is going to change the link state, and the TX scheduler needs to
>enabled or disabled accordingly.
Sorry, I think I didn't explain it clearly. The driver *does* notify
about link changes even when it has autonegotiation disabled.
The only thing that doesn't work is the notification when someone changes
it via ethtool. In the same moment.
i.e. when the other peer ifdowns the port - the driver does see it and
notifies about it.
The only scenario that doesn't work is when 8139too is manually (locally)
changed, by ethtool, to autoneg off. Then it doesn't notify anything. So,
effectively, if the administrator issues an 'ethtool -s eth0 autoneg off
speed X', bonding/bridge/etc. won't be notified about that. However, if the
cable is pulled afterwards - it will notify about link down.
Basically, when changing the interface manually to autoneg off and whatever
else, the driver doesn't issue the NETDEV_* event. That's what I've added.
Other drivers, after setting autoneg off, usually ifdown/ifup the interface
manually, and thus everything gets updated. That is another way to fix this
driver - to issue ->ndo_stop(); ->ndo_open(); (as netxen_nic/qlcnic/etc
do), but I think that just issuing a notification that we've changed the
state, without flapping, is better.
>
>Perhaps this driver needs to run a link polling timer while autoneg is
>off.
>
>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply
* Re: [PATCH] team: unsyc the devices addresses when port is removed
From: Jiri Pirko @ 2013-03-07 18:30 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1362679165-31203-1-git-send-email-vyasevic@redhat.com>
Thu, Mar 07, 2013 at 06:59:25PM CET, vyasevic@redhat.com wrote:
>When a team port is removed, unsync all devices addresses that may have
>been synched to the port devices.
This is looking good to me. Thanks for spotting this bug.
Acked-by: Jiri Pirko <jiri@resnulli.us>
>
>CC: Jiri Pirko <jiri@resnulli.us>
>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>---
> drivers/net/team/team.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index 05c5efe..bf34192 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -1138,6 +1138,8 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
> netdev_upper_dev_unlink(port_dev, dev);
> team_port_disable_netpoll(port);
> vlan_vids_del_by_dev(port_dev, dev);
>+ dev_uc_unsync(port_dev, dev);
>+ dev_mc_unsync(port_dev, dev);
> dev_close(port_dev);
> team_port_leave(team, port);
>
>--
>1.7.7.6
>
^ permalink raw reply
* Re: [PATCH v2 5/7] usbnet: qmi_wwan: comments on suspend failure
From: Bjørn Mork @ 2013-03-07 18:08 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Jiri Kosina, Alan Stern,
Oliver Neukum, netdev, linux-usb, linux-input
In-Reply-To: <1362672924-22975-6-git-send-email-ming.lei@canonical.com>
Ming Lei <ming.lei@canonical.com> writes:
> If suspend callback fails in system sleep context, usb core will
> ignore the failure and let system sleep go ahead further, so
> this patch comments on the case and requires that both
> usbnet_suspend() and subdriver->suspend() MUST return 0 in
> system sleep context.
>
> Cc: Bjørn Mork <bjorn@mork.no>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> drivers/net/usb/qmi_wwan.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index efb5c7c..8a60bdd 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -288,6 +288,11 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
> struct qmi_wwan_state *info = (void *)&dev->data;
> int ret;
>
> + /*
> + * Both usbnet_suspend() and subdriver->suspend() MUST return 0
> + * in system sleep context, otherwise, the resume callback has
> + * to recover device from previous suspend failure.
> + */
> ret = usbnet_suspend(intf, message);
> if (ret < 0)
> goto err;
Acked-by: Bjørn Mork <bjorn@mork.no>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 4/7] usbnet: cdc_mbim: comments on suspend failure
From: Bjørn Mork @ 2013-03-07 18:08 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Jiri Kosina, Alan Stern,
Oliver Neukum, netdev, linux-usb, linux-input
In-Reply-To: <1362672924-22975-5-git-send-email-ming.lei@canonical.com>
Ming Lei <ming.lei@canonical.com> writes:
> If suspend callback fails in system sleep context, usb core will
> ignore the failure and let system sleep go ahead further, so
> this patch comments on the case and requires that both
> usbnet_suspend() and subdriver->suspend() MUST return 0 in
> system sleep context.
>
> Cc: Bjørn Mork <bjorn@mork.no>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> drivers/net/usb/cdc_mbim.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
> index 248d2dc..406a34d 100644
> --- a/drivers/net/usb/cdc_mbim.c
> +++ b/drivers/net/usb/cdc_mbim.c
> @@ -332,6 +332,11 @@ static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
> goto error;
> }
>
> + /*
> + * Both usbnet_suspend() and subdriver->suspend() MUST return 0
> + * in system sleep context, otherwise, the resume callback has
> + * to recover device from previous suspend failure.
> + */
> ret = usbnet_suspend(intf, message);
> if (ret < 0)
> goto error;
Looks fine. Thanks
Acked-by: Bjørn Mork <bjorn@mork.no>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] team: unsyc the devices addresses when port is removed
From: Vlad Yasevich @ 2013-03-07 17:59 UTC (permalink / raw)
To: netdev; +Cc: Vlad Yasevich, Jiri Pirko
When a team port is removed, unsync all devices addresses that may have
been synched to the port devices.
CC: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
drivers/net/team/team.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 05c5efe..bf34192 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1138,6 +1138,8 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
netdev_upper_dev_unlink(port_dev, dev);
team_port_disable_netpoll(port);
vlan_vids_del_by_dev(port_dev, dev);
+ dev_uc_unsync(port_dev, dev);
+ dev_mc_unsync(port_dev, dev);
dev_close(port_dev);
team_port_leave(team, port);
--
1.7.7.6
^ permalink raw reply related
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
From: Vlad Yasevich @ 2013-03-07 17:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
In-Reply-To: <20130307091340.6c5af4bb@nehalam.linuxnetplumber.net>
On 03/07/2013 12:13 PM, Stephen Hemminger wrote:
> On Thu, 07 Mar 2013 10:35:37 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> On 03/07/2013 02:19 AM, Stephen Hemminger wrote:
>>> I understand the desire to add more functionality, but in this case it
>>> would introduce lots more problems. STP would break and it doesn't seem to
>>> gain anything that can't be done by other means.
>>>
>>> Turning bridge into macvlan seems unnecessary. Combining apples and bananas
>>> doesn't always make a tasty smoothy, sometimes it is just a mess.
>>>
>>> Maybe adding a little more to macvlan to do what you want would be simpler.
>>>
>>
>>
>> It's not really a macvlan over the bridge. I would agree that
>> particular setup would be a bit odd. This work enables VMs to manage
>> their mac addresses and to reduce the load on the host by keeping the
>> bridge in promisc mode.
>>
>> Sadly, most kvm network configs still use bridging and have not
>> transitioned to OVS. macvlan has some limitations as well and I working
>> to address those, but there is a desire for non-promisc bridge. In
>> this case VMs can manage their mac addresses and can write that data to
>> the bridge.
>>
>> STP is not broken as STP uses multicast mac and we set IFF_ALLMULTI thus
>> continuing to receive and process STP BPDUs.
>>
>> The one thing that would appear to suffer from this is VLAN reception,
>> but the bridge does allow vlan config now and that would have to be
>> configured if VMs wish to use vlans.
>>
>> I am not changing default operation of the bridge. Default is still
>> promisc. In fact, one can switch back and forth without any network
>> outages. This simply adds another mode the the bridge operation.
>>
>
> 1. I am not a fan of the added complexity.
> 2, Don't use sysfs for new API's use netlink instead.
Oh, I had a question for you about this. I am changing the uplink code
slightly to pattern more after some of the security features you added
(like root_block and bpdu_guard). I makes things simpler. I would
really like to provide the sysfs interface, because I checked iproute
code and I don't see any netlink implementation of those things.
Would that be more agreeable to you?
Thanks
-vlad
I
> 3. It depends on the uplink port providing UNICAST filtering which some
> physical devices don't do.
>
^ permalink raw reply
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
From: Vlad Yasevich @ 2013-03-07 17:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
In-Reply-To: <20130307091340.6c5af4bb@nehalam.linuxnetplumber.net>
On 03/07/2013 12:13 PM, Stephen Hemminger wrote:
> On Thu, 07 Mar 2013 10:35:37 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> On 03/07/2013 02:19 AM, Stephen Hemminger wrote:
>>> I understand the desire to add more functionality, but in this case it
>>> would introduce lots more problems. STP would break and it doesn't seem to
>>> gain anything that can't be done by other means.
>>>
>>> Turning bridge into macvlan seems unnecessary. Combining apples and bananas
>>> doesn't always make a tasty smoothy, sometimes it is just a mess.
>>>
>>> Maybe adding a little more to macvlan to do what you want would be simpler.
>>>
>>
>>
>> It's not really a macvlan over the bridge. I would agree that
>> particular setup would be a bit odd. This work enables VMs to manage
>> their mac addresses and to reduce the load on the host by keeping the
>> bridge in promisc mode.
>>
>> Sadly, most kvm network configs still use bridging and have not
>> transitioned to OVS. macvlan has some limitations as well and I working
>> to address those, but there is a desire for non-promisc bridge. In
>> this case VMs can manage their mac addresses and can write that data to
>> the bridge.
>>
>> STP is not broken as STP uses multicast mac and we set IFF_ALLMULTI thus
>> continuing to receive and process STP BPDUs.
>>
>> The one thing that would appear to suffer from this is VLAN reception,
>> but the bridge does allow vlan config now and that would have to be
>> configured if VMs wish to use vlans.
>>
>> I am not changing default operation of the bridge. Default is still
>> promisc. In fact, one can switch back and forth without any network
>> outages. This simply adds another mode the the bridge operation.
>>
>
> 1. I am not a fan of the added complexity.
These patches are very simple and don't impact bridge operations.
> 2, Don't use sysfs for new API's use netlink instead.
Ok, I can remove the sysfs uplink interface.
> 3. It depends on the uplink port providing UNICAST filtering which some
> physical devices don't do.
>
If the physical device doesn't provide filtering, it will be placed in
promiscuous mode. So, it essentially reverts back to the old behavior.
However, on devices that do support unicast filtering, there is a win.
-vlad
^ permalink raw reply
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
From: Stephen Hemminger @ 2013-03-07 17:13 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, bridge
In-Reply-To: <5138B3C9.4060404@redhat.com>
On Thu, 07 Mar 2013 10:35:37 -0500
Vlad Yasevich <vyasevic@redhat.com> wrote:
> On 03/07/2013 02:19 AM, Stephen Hemminger wrote:
> > I understand the desire to add more functionality, but in this case it
> > would introduce lots more problems. STP would break and it doesn't seem to
> > gain anything that can't be done by other means.
> >
> > Turning bridge into macvlan seems unnecessary. Combining apples and bananas
> > doesn't always make a tasty smoothy, sometimes it is just a mess.
> >
> > Maybe adding a little more to macvlan to do what you want would be simpler.
> >
>
>
> It's not really a macvlan over the bridge. I would agree that
> particular setup would be a bit odd. This work enables VMs to manage
> their mac addresses and to reduce the load on the host by keeping the
> bridge in promisc mode.
>
> Sadly, most kvm network configs still use bridging and have not
> transitioned to OVS. macvlan has some limitations as well and I working
> to address those, but there is a desire for non-promisc bridge. In
> this case VMs can manage their mac addresses and can write that data to
> the bridge.
>
> STP is not broken as STP uses multicast mac and we set IFF_ALLMULTI thus
> continuing to receive and process STP BPDUs.
>
> The one thing that would appear to suffer from this is VLAN reception,
> but the bridge does allow vlan config now and that would have to be
> configured if VMs wish to use vlans.
>
> I am not changing default operation of the bridge. Default is still
> promisc. In fact, one can switch back and forth without any network
> outages. This simply adds another mode the the bridge operation.
>
1. I am not a fan of the added complexity.
2, Don't use sysfs for new API's use netlink instead.
3. It depends on the uplink port providing UNICAST filtering which some
physical devices don't do.
^ permalink raw reply
* Re: [PATCH 2/3] arm: dts: am33xx: add default ethtool slave to cpsw node
From: Mugunthan V N @ 2013-03-07 17:13 UTC (permalink / raw)
To: Tony Lindgren; +Cc: netdev, devicetree-discuss, linux-omap, davem
In-Reply-To: <20130307161308.GW11806@atomide.com>
On 3/7/2013 9:43 PM, Tony Lindgren wrote:
> * Mugunthan V N <mugunthanvnm@ti.com> [130307 04:35]:
>
> Can you please add a description and send this separately
> so Benoit can queue it.
>
>
Will send this patch separately after this patch series has been
reviewed and accepted by netdev maintainer.
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
From: Mugunthan V N @ 2013-03-07 17:13 UTC (permalink / raw)
To: Peter Korsgaard
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-omap-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <8762135on4.fsf-D6SC8u56vOOJDPpyT6T3/w@public.gmane.org>
On 3/7/2013 6:54 PM, Peter Korsgaard wrote:
>>>>>> "M" == Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> writes:
> M> This patch implements get/set of the phy settings via ethtool apis
> M> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> M> ---
> M> Documentation/devicetree/bindings/net/cpsw.txt | 3 +++
> M> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++++++
> M> include/linux/platform_data/cpsw.h | 1 +
> M> 3 files changed, 36 insertions(+)
>
> M> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> M> index ecfdf75..8d61300 100644
> M> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> M> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> M> @@ -20,6 +20,7 @@ Required properties:
> M> - cpts_clock_shift : Denominator to convert input clock ticks into nanoseconds
> M> - phy_id : Specifies slave phy id
> M> - mac-address : Specifies slave MAC address
> M> +- ethtool-active-slave : Specifies the slave to use for ethtool command
>
> That again sounds like something Linux specific rather than a hardware
> property.
>
> It would be good if all these special things (dual emac mode, vlan
> handling, switching) could be handled using the existing kernel
> (bridging/vlan) infrastructure, and the driver always just exposing 2
> network interfaces instead of these configuration properties.
>
Switch and Dual Emac modes of operation of CPSW are two different
features of the
hardware and packet routing between the slaves in the hardware are
different in
both the modes.
If by default it is brought up as Dual EMAC then hardware switching is
blocked and
use-cases like IP phone etc cannot be achieved.
Since CPSW as a hardware Switch, it cannot not be handled in existing kernel
feature.
Regards
Mugunthan V N
^ permalink raw reply
* Re: [RFC/RFT 00/27] Updates for the rtlwifi family of drivers
From: Larry Finger @ 2013-03-07 17:01 UTC (permalink / raw)
To: Matt Chen
Cc: Joe Perches, John Linville, linux-wireless, netdev,
Jeffrey Cheung, Michal Marek
In-Reply-To: <CALx5=V_Y2=bZWXzc9uQkmTBAECeMLbnPU0VA56Wp2tNeBB3Dqw@mail.gmail.com>
On 03/06/2013 10:49 PM, Matt Chen wrote:
> I tried and test the patches for rtl8188ee and rtl8192ce, so far are working
> fine to me. :)
>
> Acked-by: Chen, Chien-Chia <machen@suse.com <mailto:machen@suse.com>>
Thanks for the update. I am still chasing system crashes related to CPU lockups
caused by spinlock contention. I take it that you have not seen these.
Larry
^ permalink raw reply
* Re: [PATCH] VSOCK: Split vm_sockets.h into kernel/uapi
From: David Howells @ 2013-03-07 16:58 UTC (permalink / raw)
To: Andy King
Cc: dhowells, netdev, linux-kernel, virtualization, gregkh, davem,
pv-drivers
In-Reply-To: <1362669973-26984-1-git-send-email-acking@vmware.com>
Andy King <acking@vmware.com> wrote:
> Split the vSockets header into kernel and UAPI parts. The former gets the bits
> that used to be in __KERNEL__ guards, while the latter gets everything that is
> user-visible. Tested by compiling vsock (+transport) and a simple user-mode
> vSockets application.
>
> Reported-by: David Howells <dhowells@redhat.com>
> Acked-by: Dmitry Torokhov <dtor@vmware.com>
> Signed-off-by: Andy King <acking@vmware.com>
Acked-by: David Howells <dhowells@redhat.com>
^ permalink raw reply
* Re: [PATCH] 8139too: send NETDEV_CHANGE manually when autoneg is disabled
From: Ben Hutchings @ 2013-03-07 16:54 UTC (permalink / raw)
To: Veaceslav Falico
Cc: David Miller, netdev, wfp5p, jasowang, junchangwang, greearb,
ivecera
In-Reply-To: <20130307163537.GA21810@redhat.com>
On Thu, 2013-03-07 at 17:35 +0100, Veaceslav Falico wrote:
> On Thu, Mar 07, 2013 at 03:52:35PM +0000, Ben Hutchings wrote:
> >On Thu, 2013-03-07 at 11:27 +0100, Veaceslav Falico wrote:
> >> On Wed, Mar 06, 2013 at 03:53:23PM -0500, David Miller wrote:
> >> >From: Ben Hutchings <bhutchings@solarflare.com>
> >> >Date: Wed, 6 Mar 2013 20:22:52 +0000
> >> >
> >> >> On Wed, 2013-03-06 at 20:06 +0100, Veaceslav Falico wrote:
> >> >>> When setting autoneg off (with any additional parameters, like
> >> >>> speed/duplex), 8139too doesn't do an interface reset, and thus doesn't
> >> >>> notify anyone that its speed/duplex might have changed (bonding and bridge
> >> >>> will not see the speed changes, per example).
> >> >>>
> >> >>> Verify if we've force_media and send notification manually, so that the
> >> >>> listeners have a chance to see the changes. It's quite ugly, however I
> >> >>> don't see anything better.
> >> >>
> >> >> Isn't this really a bug in mii_check_media()? It shouldn't shortcut the
> >> >> calls to netif_carrier_{off,on}() just because mii->force_media is set.
> >> >
> >> >I think mii_check_media() is responsible for handling this too.
> >>
> >> The mii_check_media() doesn't get called, AFAIK. The problem here is that,
> >> after we call ethtool -s eth0 autoneg off speed X, with eth0 being
> >> 8139too, the speed/autoneg options are changed via mii_ethtool_sset(),
> >> however the interface itself isn't down'ed/up'ed, and thus no NETDEV_
> >> notifications are sent.
> >
> >Does the hardware not send link interrupts if autoneg is disabled?
>
> Yes, it doesn't send them when the autoneg off is set.
>
> >
> >> Other drivers either explicitly reset the interface after
> >> ethtool.set_settings() call (like netxen_nic using ndo_close()/ndo_open()),
> >> do it on the logic level (tg3) without involving mii_ethtool_sset(), or
> >> just reset on their own (e100 iirc), so that most of them are responsible
> >> for somehow triggering these events.
> >>
> >> Silently changing speed can break things a bit - bonding relies on
> >> interface speeds for 802.3ad/alb/tlb/active-backup iirc, bridge relies on
> >> stp port cost etc. and they all get it via NETDEV_ notifications. So
> >> without them, they would end up with outdated data, per example (eth2 being
> >> 8139too):
> >[...]
> >
> >Yes, I get it. But on real hardware, changing speed/duplex is always
> >going to break the link if it's up. The link change notification should
> >result in kernel and userland notifications via linkwatch_do_dev().
> >
> >(If you're testing against QEMU rather than real hardware, there could
> >be a bug in the emulation of link interrupts.)
>
> It's real hardware:
>
> [ 4.339413] 8139cp 0000:10:09.0: This (id 10ec:8139 rev 10) is not an 8139C+ compatible chip, use 8139too
> [ 4.348210] 8139too: 8139too Fast Ethernet driver 0.9.28
> [ 4.350017] 8139too 0000:10:09.0 eth2: RealTek RTL8139 at 0xffffc90004574100, 00:14:d1:1f:b9:49, IRQ 21
[...]
OK. But it's generally not enough to send a 'something changed'
notification; the driver actually has to update the link state. MAybe
in your test you reconfigure the other end of the link too, or it's
smart enough to do parallel detect. But in general, changing the link
speed is going to change the link state, and the TX scheduler needs to
enabled or disabled accordingly.
Perhaps this driver needs to run a link polling timer while autoneg is
off.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] 8139too: send NETDEV_CHANGE manually when autoneg is disabled
From: Veaceslav Falico @ 2013-03-07 16:35 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Miller, netdev, wfp5p, jasowang, junchangwang, greearb,
ivecera
In-Reply-To: <1362671555.2936.10.camel@bwh-desktop.uk.solarflarecom.com>
On Thu, Mar 07, 2013 at 03:52:35PM +0000, Ben Hutchings wrote:
>On Thu, 2013-03-07 at 11:27 +0100, Veaceslav Falico wrote:
>> On Wed, Mar 06, 2013 at 03:53:23PM -0500, David Miller wrote:
>> >From: Ben Hutchings <bhutchings@solarflare.com>
>> >Date: Wed, 6 Mar 2013 20:22:52 +0000
>> >
>> >> On Wed, 2013-03-06 at 20:06 +0100, Veaceslav Falico wrote:
>> >>> When setting autoneg off (with any additional parameters, like
>> >>> speed/duplex), 8139too doesn't do an interface reset, and thus doesn't
>> >>> notify anyone that its speed/duplex might have changed (bonding and bridge
>> >>> will not see the speed changes, per example).
>> >>>
>> >>> Verify if we've force_media and send notification manually, so that the
>> >>> listeners have a chance to see the changes. It's quite ugly, however I
>> >>> don't see anything better.
>> >>
>> >> Isn't this really a bug in mii_check_media()? It shouldn't shortcut the
>> >> calls to netif_carrier_{off,on}() just because mii->force_media is set.
>> >
>> >I think mii_check_media() is responsible for handling this too.
>>
>> The mii_check_media() doesn't get called, AFAIK. The problem here is that,
>> after we call ethtool -s eth0 autoneg off speed X, with eth0 being
>> 8139too, the speed/autoneg options are changed via mii_ethtool_sset(),
>> however the interface itself isn't down'ed/up'ed, and thus no NETDEV_
>> notifications are sent.
>
>Does the hardware not send link interrupts if autoneg is disabled?
Yes, it doesn't send them when the autoneg off is set.
>
>> Other drivers either explicitly reset the interface after
>> ethtool.set_settings() call (like netxen_nic using ndo_close()/ndo_open()),
>> do it on the logic level (tg3) without involving mii_ethtool_sset(), or
>> just reset on their own (e100 iirc), so that most of them are responsible
>> for somehow triggering these events.
>>
>> Silently changing speed can break things a bit - bonding relies on
>> interface speeds for 802.3ad/alb/tlb/active-backup iirc, bridge relies on
>> stp port cost etc. and they all get it via NETDEV_ notifications. So
>> without them, they would end up with outdated data, per example (eth2 being
>> 8139too):
>[...]
>
>Yes, I get it. But on real hardware, changing speed/duplex is always
>going to break the link if it's up. The link change notification should
>result in kernel and userland notifications via linkwatch_do_dev().
>
>(If you're testing against QEMU rather than real hardware, there could
>be a bug in the emulation of link interrupts.)
It's real hardware:
[ 4.339413] 8139cp 0000:10:09.0: This (id 10ec:8139 rev 10) is not an 8139C+ compatible chip, use 8139too
[ 4.348210] 8139too: 8139too Fast Ethernet driver 0.9.28
[ 4.350017] 8139too 0000:10:09.0 eth2: RealTek RTL8139 at 0xffffc90004574100, 00:14:d1:1f:b9:49, IRQ 21
darkmag:~#ethtool eth2
Settings for eth2:
<snip>
Supports auto-negotiation: Yes
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
Advertised pause frame use: No
Advertised auto-negotiation: Yes
Link partner advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
Link partner advertised pause frame use: Symmetric
Link partner advertised auto-negotiation: Yes
Speed: 100Mb/s
Duplex: Full
<snip>
darkmag:~#dmesg | tail -n 10 | grep 8139too
[ 164.019622] 8139too 0000:10:09.0 eth2: link up, 100Mbps, full-duplex, lpa 0x45E1
darkmag:~#ethtool -s eth2 autoneg off speed 10
darkmag:~#dmesg | tail -n 10 | grep 8139too
[ 164.019622] 8139too 0000:10:09.0 eth2: link up, 100Mbps, full-duplex, lpa 0x45E1
darkmag:~#ethtool eth2
Settings for eth2:
<snip>
Supports auto-negotiation: Yes
Advertised link modes: Not reported
Advertised pause frame use: No
Advertised auto-negotiation: No
Speed: 10Mb/s
Duplex: Full
And, obviously, bonding's not updated:
darkmag:~#grep 'Interface\|Speed' /proc/net/bonding/bond0
Slave Interface: eth0
Speed: 100 Mbps
Slave Interface: eth2
Speed: 100 Mbps
>
>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply
* [PATCH v2 7/7] usbnet: smsc75xx: don't recover device if suspend fails in system sleep
From: Ming Lei @ 2013-03-07 16:15 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev, linux-usb, linux-input,
Ming Lei, Steve Glendinning
In-Reply-To: <1362672924-22975-1-git-send-email-ming.lei@canonical.com>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch doesn't recover device under this situation.
Also add comments on this case.
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/smsc75xx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 9abe517..21b607a 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2011,7 +2011,11 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
ret = smsc75xx_enter_suspend0(dev);
done:
- if (ret)
+ /*
+ * TODO: resume() might need to handle the suspend failure
+ * in system sleep
+ */
+ if (ret && PMSG_IS_AUTO(message))
usbnet_resume(intf);
return ret;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 6/7] usbnet: smsc95xx: don't recover device if suspend fails in system sleep
From: Ming Lei @ 2013-03-07 16:15 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev, linux-usb, linux-input,
Ming Lei, Steve Glendinning
In-Reply-To: <1362672924-22975-1-git-send-email-ming.lei@canonical.com>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch doesn't recover device under this situation.
Also add comments on the case.
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/smsc95xx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index e6d2dea..3f38ba8 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1660,7 +1660,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
ret = smsc95xx_enter_suspend0(dev);
done:
- if (ret)
+ /*
+ * TODO: resume() might need to handle the suspend failure
+ * in system sleep
+ */
+ if (ret && PMSG_IS_AUTO(message))
usbnet_resume(intf);
return ret;
}
--
1.7.9.5
^ 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