* [PATCH net v2 0/2] macvlan: disable LRO on lowerdev instead of a macvlan
@ 2013-11-14 14:00 Michal Kubecek
2013-11-14 14:00 ` [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function Michal Kubecek
2013-11-14 14:00 ` [PATCH net v2 2/2] macvlan: disable LRO on lower device instead of macvlan Michal Kubecek
0 siblings, 2 replies; 8+ messages in thread
From: Michal Kubecek @ 2013-11-14 14:00 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Patrick McHardy, John Fastabend
A customer of ours encountered a problem with LRO on an ixgbe network
card. Analysis showed that it was a known conflict of forwarding and LRO
but the forwarding was enabled in an LXC container where only a macvlan
was, not the ethernet device itself.
I believe the solution is exactly the same as what we do for "normal"
(802.1q) VLAN devices: if dev_disable_lro() is called for such device,
LRO is disabled on the underlying "real" device instead.
v2: adapt to changes merged from net-next
Michal Kubecek (2):
macvlan: introduce macvlan_dev_real_dev() helper function
macvlan: disable LRO on lower device instead of macvlan
include/linux/if_macvlan.h | 16 ++++++++++++++++
net/core/dev.c | 5 +++++
2 files changed, 21 insertions(+)
--
1.8.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function
2013-11-14 14:00 [PATCH net v2 0/2] macvlan: disable LRO on lowerdev instead of a macvlan Michal Kubecek
@ 2013-11-14 14:00 ` Michal Kubecek
2013-11-14 15:03 ` Vlad Yasevich
2013-11-14 14:00 ` [PATCH net v2 2/2] macvlan: disable LRO on lower device instead of macvlan Michal Kubecek
1 sibling, 1 reply; 8+ messages in thread
From: Michal Kubecek @ 2013-11-14 14:00 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Patrick McHardy, John Fastabend
Introduce helper function macvlan_dev_real_dev which returns the
underlying device of a macvlan device, similar to vlan_dev_real_dev()
for 802.1q VLAN devices.
v2: IFF_MACVLAN flag and equivalent of is_macvlan_dev() were
introduced in the meantime
Signed-off-by: Michal Kubecek <mkubecek@suse•cz>
---
include/linux/if_macvlan.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index c270285..ac9aab2 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -119,4 +119,20 @@ extern int macvlan_link_register(struct rtnl_link_ops *ops);
extern netdev_tx_t macvlan_start_xmit(struct sk_buff *skb,
struct net_device *dev);
+#if IS_ENABLED(CONFIG_MACVLAN)
+static inline struct net_device *
+macvlan_dev_real_dev(const struct net_device *dev)
+{
+ struct macvlan_dev *macvlan = netdev_priv(dev);
+
+ return macvlan->lowerdev;
+}
+#else
+static inline struct net_device *
+macvlan_dev_real_dev(const struct net_device *dev)
+{
+ return NULL;
+}
+#endif
+
#endif /* _LINUX_IF_MACVLAN_H */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function
2013-11-14 14:00 ` [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function Michal Kubecek
@ 2013-11-14 15:03 ` Vlad Yasevich
2013-11-14 15:57 ` Michal Kubecek
0 siblings, 1 reply; 8+ messages in thread
From: Vlad Yasevich @ 2013-11-14 15:03 UTC (permalink / raw)
To: Michal Kubecek, netdev; +Cc: David S. Miller, Patrick McHardy, John Fastabend
On 11/14/2013 09:00 AM, Michal Kubecek wrote:
> Introduce helper function macvlan_dev_real_dev which returns the
> underlying device of a macvlan device, similar to vlan_dev_real_dev()
> for 802.1q VLAN devices.
>
> v2: IFF_MACVLAN flag and equivalent of is_macvlan_dev() were
> introduced in the meantime
>
> Signed-off-by: Michal Kubecek <mkubecek@suse•cz>
> ---
> include/linux/if_macvlan.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
> index c270285..ac9aab2 100644
> --- a/include/linux/if_macvlan.h
> +++ b/include/linux/if_macvlan.h
> @@ -119,4 +119,20 @@ extern int macvlan_link_register(struct rtnl_link_ops *ops);
> extern netdev_tx_t macvlan_start_xmit(struct sk_buff *skb,
> struct net_device *dev);
>
> +#if IS_ENABLED(CONFIG_MACVLAN)
> +static inline struct net_device *
> +macvlan_dev_real_dev(const struct net_device *dev)
> +{
> + struct macvlan_dev *macvlan = netdev_priv(dev);
> +
> + return macvlan->lowerdev;
> +}
> +#else
> +static inline struct net_device *
> +macvlan_dev_real_dev(const struct net_device *dev)
> +{
> + return NULL;
> +}
> +#endif
> +
You may want to do the same here as was done for vlan_dev_real_dev().
This function is not intended to be called blindly and should always
be called after netif_is_macvlan().
-vlad
> #endif /* _LINUX_IF_MACVLAN_H */
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function
2013-11-14 15:03 ` Vlad Yasevich
@ 2013-11-14 15:57 ` Michal Kubecek
2013-11-14 22:03 ` David Miller
2013-11-15 2:43 ` Vlad Yasevich
0 siblings, 2 replies; 8+ messages in thread
From: Michal Kubecek @ 2013-11-14 15:57 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, David S. Miller, Patrick McHardy, John Fastabend
On Thu, Nov 14, 2013 at 10:03:19AM -0500, Vlad Yasevich wrote:
> On 11/14/2013 09:00 AM, Michal Kubecek wrote:
> >+#if IS_ENABLED(CONFIG_MACVLAN)
> >+static inline struct net_device *
> >+macvlan_dev_real_dev(const struct net_device *dev)
> >+{
> >+ struct macvlan_dev *macvlan = netdev_priv(dev);
> >+
> >+ return macvlan->lowerdev;
> >+}
> >+#else
> >+static inline struct net_device *
> >+macvlan_dev_real_dev(const struct net_device *dev)
> >+{
> >+ return NULL;
> >+}
> >+#endif
> >+
>
> You may want to do the same here as was done for
> vlan_dev_real_dev(). This function is not intended to be called
> blindly and should always
> be called after netif_is_macvlan().
I'm not sure. It makes sense from the developer point of view: if we
find an inconsistency which must be caused by a bug in kernel code, do
panic so that the bug is found and fixed as soon as possible. However,
I remember a discussion where the point was that BUG() and BUG_ON()
should only be used if there is no way to recover. From this point of
view, WARN or WARN_ONCE might be better choice - but I'm not strictly
opposed to BUG().
Michal Kubecek
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function
2013-11-14 15:57 ` Michal Kubecek
@ 2013-11-14 22:03 ` David Miller
2013-11-15 5:26 ` Michal Kubecek
2013-11-15 2:43 ` Vlad Yasevich
1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2013-11-14 22:03 UTC (permalink / raw)
To: mkubecek; +Cc: vyasevich, netdev, kaber, john.r.fastabend
From: Michal Kubecek <mkubecek@suse•cz>
Date: Thu, 14 Nov 2013 16:57:57 +0100
> On Thu, Nov 14, 2013 at 10:03:19AM -0500, Vlad Yasevich wrote:
>> On 11/14/2013 09:00 AM, Michal Kubecek wrote:
>> >+#if IS_ENABLED(CONFIG_MACVLAN)
>> >+static inline struct net_device *
>> >+macvlan_dev_real_dev(const struct net_device *dev)
>> >+{
>> >+ struct macvlan_dev *macvlan = netdev_priv(dev);
>> >+
>> >+ return macvlan->lowerdev;
>> >+}
>> >+#else
>> >+static inline struct net_device *
>> >+macvlan_dev_real_dev(const struct net_device *dev)
>> >+{
>> >+ return NULL;
>> >+}
>> >+#endif
>> >+
>>
>> You may want to do the same here as was done for
>> vlan_dev_real_dev(). This function is not intended to be called
>> blindly and should always
>> be called after netif_is_macvlan().
>
> I'm not sure. It makes sense from the developer point of view: if we
> find an inconsistency which must be caused by a bug in kernel code, do
> panic so that the bug is found and fixed as soon as possible. However,
> I remember a discussion where the point was that BUG() and BUG_ON()
> should only be used if there is no way to recover. From this point of
> view, WARN or WARN_ONCE might be better choice - but I'm not strictly
> opposed to BUG().
At least for the time being use BUG(), to be consistent with the same
way how VLAN handles this situation.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function
2013-11-14 22:03 ` David Miller
@ 2013-11-15 5:26 ` Michal Kubecek
0 siblings, 0 replies; 8+ messages in thread
From: Michal Kubecek @ 2013-11-15 5:26 UTC (permalink / raw)
To: David Miller; +Cc: vyasevich, netdev, kaber, john.r.fastabend
On Thu, Nov 14, 2013 at 05:03:58PM -0500, David Miller wrote:
> >> On 11/14/2013 09:00 AM, Michal Kubecek wrote:
> >> >+#if IS_ENABLED(CONFIG_MACVLAN)
> >> >+static inline struct net_device *
> >> >+macvlan_dev_real_dev(const struct net_device *dev)
> >> >+{
> >> >+ struct macvlan_dev *macvlan = netdev_priv(dev);
> >> >+
> >> >+ return macvlan->lowerdev;
> >> >+}
> >> >+#else
> >> >+static inline struct net_device *
> >> >+macvlan_dev_real_dev(const struct net_device *dev)
> >> >+{
> >> >+ return NULL;
> >> >+}
> >> >+#endif
> >> >+
>
> At least for the time being use BUG(), to be consistent with the same
> way how VLAN handles this situation.
OK, v3 sent.
Michal Kubecek
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function
2013-11-14 15:57 ` Michal Kubecek
2013-11-14 22:03 ` David Miller
@ 2013-11-15 2:43 ` Vlad Yasevich
1 sibling, 0 replies; 8+ messages in thread
From: Vlad Yasevich @ 2013-11-15 2:43 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, David S. Miller, Patrick McHardy, John Fastabend
On 11/14/2013 10:57 AM, Michal Kubecek wrote:
> On Thu, Nov 14, 2013 at 10:03:19AM -0500, Vlad Yasevich wrote:
>> On 11/14/2013 09:00 AM, Michal Kubecek wrote:
>>> +#if IS_ENABLED(CONFIG_MACVLAN)
>>> +static inline struct net_device *
>>> +macvlan_dev_real_dev(const struct net_device *dev)
>>> +{
>>> + struct macvlan_dev *macvlan = netdev_priv(dev);
>>> +
>>> + return macvlan->lowerdev;
>>> +}
>>> +#else
>>> +static inline struct net_device *
>>> +macvlan_dev_real_dev(const struct net_device *dev)
>>> +{
>>> + return NULL;
>>> +}
>>> +#endif
>>> +
>>
>> You may want to do the same here as was done for
>> vlan_dev_real_dev(). This function is not intended to be called
>> blindly and should always
>> be called after netif_is_macvlan().
>
> I'm not sure. It makes sense from the developer point of view: if we
> find an inconsistency which must be caused by a bug in kernel code, do
> panic so that the bug is found and fixed as soon as possible. However,
> I remember a discussion where the point was that BUG() and BUG_ON()
> should only be used if there is no way to recover. From this point of
> view, WARN or WARN_ONCE might be better choice - but I'm not strictly
> opposed to BUG().
>
> Michal Kubecek
>
If someone blindly calls macvlan_dev_real_dev() and macvlan module is
enabled, but there is no macvlan device, you are going to get garbage
and crash. So crashing if module is disabled for the same case seems
perfectly reasonable.
-vlad
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v2 2/2] macvlan: disable LRO on lower device instead of macvlan
2013-11-14 14:00 [PATCH net v2 0/2] macvlan: disable LRO on lowerdev instead of a macvlan Michal Kubecek
2013-11-14 14:00 ` [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function Michal Kubecek
@ 2013-11-14 14:00 ` Michal Kubecek
1 sibling, 0 replies; 8+ messages in thread
From: Michal Kubecek @ 2013-11-14 14:00 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Patrick McHardy, John Fastabend
A macvlan device has always LRO disabled so that calling
dev_disable_lro() on it does nothing. If we need to disable LRO
e.g. because
- the macvlan device is inserted into a bridge
- IPv6 forwarding is enabled for it
- it is in a different namespace than lowerdev and IPv4
forwarding is enabled in it
we need to disable LRO on its underlying device instead (as we
do for 802.1q VLAN devices).
v2: use newly introduced netif_is_macvlan()
Signed-off-by: Michal Kubecek <mkubecek@suse•cz>
---
net/core/dev.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 974143d..7e00a73 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -131,6 +131,7 @@
#include <linux/static_key.h>
#include <linux/hashtable.h>
#include <linux/vmalloc.h>
+#include <linux/if_macvlan.h>
#include "net-sysfs.h"
@@ -1424,6 +1425,10 @@ void dev_disable_lro(struct net_device *dev)
if (is_vlan_dev(dev))
dev = vlan_dev_real_dev(dev);
+ /* the same for macvlan devices */
+ if (netif_is_macvlan(dev))
+ dev = macvlan_dev_real_dev(dev);
+
dev->wanted_features &= ~NETIF_F_LRO;
netdev_update_features(dev);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-11-15 5:26 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-14 14:00 [PATCH net v2 0/2] macvlan: disable LRO on lowerdev instead of a macvlan Michal Kubecek
2013-11-14 14:00 ` [PATCH net v2 1/2] macvlan: introduce macvlan_dev_real_dev() helper function Michal Kubecek
2013-11-14 15:03 ` Vlad Yasevich
2013-11-14 15:57 ` Michal Kubecek
2013-11-14 22:03 ` David Miller
2013-11-15 5:26 ` Michal Kubecek
2013-11-15 2:43 ` Vlad Yasevich
2013-11-14 14:00 ` [PATCH net v2 2/2] macvlan: disable LRO on lower device instead of macvlan Michal Kubecek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox