| Message ID | 1442510338-10180-1-git-send-email-linville@tuxdriver.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Sep 17, 2015 at 10:18 AM, John W. Linville <linville@tuxdriver.com> wrote: > These seem to have been accidentally dropped in commit 371bd1061d29 > ("geneve: Consolidate Geneve functionality in single module."). > Geneve should not export vxlan feature. So that it never sees vxlan tagged packets. Can you turn off the vlan feature? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 17, 2015 at 11:45:58AM -0700, Pravin Shelar wrote: > On Thu, Sep 17, 2015 at 10:18 AM, John W. Linville > <linville@tuxdriver.com> wrote: > > These seem to have been accidentally dropped in commit 371bd1061d29 > > ("geneve: Consolidate Geneve functionality in single module."). > > > Geneve should not export vxlan feature. So that it never sees vxlan > tagged packets. Can you turn off the vlan feature? I'm not sure I understand...? This is vlan, not vxlan. John
On Thu, Sep 17, 2015 at 12:25 PM, John W. Linville <linville@tuxdriver.com> wrote: > On Thu, Sep 17, 2015 at 11:45:58AM -0700, Pravin Shelar wrote: >> On Thu, Sep 17, 2015 at 10:18 AM, John W. Linville >> <linville@tuxdriver.com> wrote: >> > These seem to have been accidentally dropped in commit 371bd1061d29 >> > ("geneve: Consolidate Geneve functionality in single module."). >> > >> Geneve should not export vxlan feature. So that it never sees vxlan >> tagged packets. Can you turn off the vlan feature? > > I'm not sure I understand...? This is vlan, not vxlan. I think he just mean vlan. If you remove the line where dev->vlan_features are set then the core stack will handle this and we don't need to do anything special here. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 17, 2015 at 12:48 PM, Jesse Gross <jesse@nicira.com> wrote: > On Thu, Sep 17, 2015 at 12:25 PM, John W. Linville > <linville@tuxdriver.com> wrote: >> On Thu, Sep 17, 2015 at 11:45:58AM -0700, Pravin Shelar wrote: >>> On Thu, Sep 17, 2015 at 10:18 AM, John W. Linville >>> <linville@tuxdriver.com> wrote: >>> > These seem to have been accidentally dropped in commit 371bd1061d29 >>> > ("geneve: Consolidate Geneve functionality in single module."). >>> > >>> Geneve should not export vxlan feature. So that it never sees vxlan >>> tagged packets. Can you turn off the vlan feature? >> >> I'm not sure I understand...? This is vlan, not vxlan. > > I think he just mean vlan. If you remove the line where > dev->vlan_features are set then the core stack will handle this and we > don't need to do anything special here. Yes, I meant vlan, sorry for confusion. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 17, 2015 at 12:48:56PM -0700, Jesse Gross wrote: > On Thu, Sep 17, 2015 at 12:25 PM, John W. Linville > <linville@tuxdriver.com> wrote: > > On Thu, Sep 17, 2015 at 11:45:58AM -0700, Pravin Shelar wrote: > >> On Thu, Sep 17, 2015 at 10:18 AM, John W. Linville > >> <linville@tuxdriver.com> wrote: > >> > These seem to have been accidentally dropped in commit 371bd1061d29 > >> > ("geneve: Consolidate Geneve functionality in single module."). > >> > > >> Geneve should not export vxlan feature. So that it never sees vxlan > >> tagged packets. Can you turn off the vlan feature? > > > > I'm not sure I understand...? This is vlan, not vxlan. > > I think he just mean vlan. If you remove the line where > dev->vlan_features are set then the core stack will handle this and we > don't need to do anything special here. Is that preferrable to this patch? Tunneling vlan-tagged frames seems weird, but I would hate to disallow it if some crazy person wanted to do that... I guess the other way would slightly improve performance, and this could be added back later. What about the VLAN-related bits in dev->features and ->hw_features? Should they go as well? John
On Thu, Sep 17, 2015 at 1:15 PM, John W. Linville <linville@tuxdriver.com> wrote: > On Thu, Sep 17, 2015 at 12:48:56PM -0700, Jesse Gross wrote: >> On Thu, Sep 17, 2015 at 12:25 PM, John W. Linville >> <linville@tuxdriver.com> wrote: >> > On Thu, Sep 17, 2015 at 11:45:58AM -0700, Pravin Shelar wrote: >> >> On Thu, Sep 17, 2015 at 10:18 AM, John W. Linville >> >> <linville@tuxdriver.com> wrote: >> >> > These seem to have been accidentally dropped in commit 371bd1061d29 >> >> > ("geneve: Consolidate Geneve functionality in single module."). >> >> > >> >> Geneve should not export vxlan feature. So that it never sees vxlan >> >> tagged packets. Can you turn off the vlan feature? >> > >> > I'm not sure I understand...? This is vlan, not vxlan. >> >> I think he just mean vlan. If you remove the line where >> dev->vlan_features are set then the core stack will handle this and we >> don't need to do anything special here. > > Is that preferrable to this patch? Tunneling vlan-tagged frames > seems weird, but I would hate to disallow it if some crazy person > wanted to do that... > To support vlan-offload in geneve we end up doing same as networking stack does in case of software fallback for vlan-offload. So by not setting the feature, we can avoid the duplicate code in geneve module. > I guess the other way would slightly improve performance, and this > could be added back later. What about the VLAN-related bits in > dev->features and ->hw_features? Should they go as well? > No need to set vlan feature in any of device features. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 17, 2015 at 1:15 PM, John W. Linville <linville@tuxdriver.com> wrote: > On Thu, Sep 17, 2015 at 12:48:56PM -0700, Jesse Gross wrote: >> On Thu, Sep 17, 2015 at 12:25 PM, John W. Linville >> <linville@tuxdriver.com> wrote: >> > On Thu, Sep 17, 2015 at 11:45:58AM -0700, Pravin Shelar wrote: >> >> On Thu, Sep 17, 2015 at 10:18 AM, John W. Linville >> >> <linville@tuxdriver.com> wrote: >> >> > These seem to have been accidentally dropped in commit 371bd1061d29 >> >> > ("geneve: Consolidate Geneve functionality in single module."). >> >> > >> >> Geneve should not export vxlan feature. So that it never sees vxlan >> >> tagged packets. Can you turn off the vlan feature? >> > >> > I'm not sure I understand...? This is vlan, not vxlan. >> >> I think he just mean vlan. If you remove the line where >> dev->vlan_features are set then the core stack will handle this and we >> don't need to do anything special here. > > Is that preferrable to this patch? Tunneling vlan-tagged frames > seems weird, but I would hate to disallow it if some crazy person > wanted to do that... This doesn't actually prevent sending VLAN tagged frames inside the tunnel. It just means that the stack won't send VLAN accelerated frames to the driver and will put them directly in the frame instead. That's the same as what this code here is emulating so there's no difference, just code cleanliness. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c index a917ae1cfbf3..0aaf302cc31b 100644 --- a/drivers/net/geneve.c +++ b/drivers/net/geneve.c @@ -14,6 +14,7 @@ #include <linux/module.h> #include <linux/netdevice.h> #include <linux/etherdevice.h> +#include <linux/if_vlan.h> #include <linux/hash.h> #include <net/dst_metadata.h> #include <net/gro_cells.h> @@ -532,13 +533,20 @@ static int geneve_build_skb(struct rtable *rt, struct sk_buff *skb, int err; min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len - + GENEVE_BASE_HLEN + opt_len + sizeof(struct iphdr); + + GENEVE_BASE_HLEN + opt_len + sizeof(struct iphdr) + + (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0); err = skb_cow_head(skb, min_headroom); if (unlikely(err)) { kfree_skb(skb); goto free_rt; } + skb = vlan_hwaccel_push_inside(skb); + if (unlikely(!skb)) { + err = -ENOMEM; + goto free_rt; + } + skb = udp_tunnel_handle_offloads(skb, csum); if (IS_ERR(skb)) { err = PTR_ERR(skb);
These seem to have been accidentally dropped in commit 371bd1061d29 ("geneve: Consolidate Geneve functionality in single module."). Signed-off-by: John W. Linville <linville@tuxdriver.com> --- drivers/net/geneve.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)