Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Fri, 1 Jun 2018 11:23:12 -0700, William Tu wrote: > Looking at the dpif_netlink_rtnl_probe_oot_tunnels(), since now we > added ERSPAN feature, instead of probing geneve module, > we should probe ip_gre module with a nlattr of ERSPAN (ex: HWID). > If it does not return -ENOSUPPORT, then use the upstream ip_gre module. That doesn't make sense. That function is used solely to determine whether the out of (kernel) tree modules are loaded. If they are, ovs prefers them. Note that the features do not come into play here: if there are out of tree modules present, they are used. It's very well possible that the running kernel has more features but it's irrelevant. It doesn't matter in which way the code determines whether there are out of tree modules loaded or not. Probing for geneve thus works just fine and there's no need to change it. > And the next added feature should change this function to determine > whether to use compat mode or not. Do I understand it right? No. The out of tree modules are preferred, irrespective of the supported features. Jiri ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Fri, Jun 01, 2018 at 06:38:53PM +0200, Jiri Benc wrote: > On Fri, 1 Jun 2018 09:15:33 -0700, Gregory Rose wrote: > > Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the > > compat interface is needed > > for kernels up to 4.15 so that we can support ERSPAN. If the built-in > > gre/ip_gre kernel modules > > don't have the ERSPAN support in them then we have to use the compat > > interface. > > That's very wrong. The compat interface should not be used with > upstream kernel (except perhaps for very very very old kernels). We > converted the API to the standard rtnetlink for good reasons. New > features are not supported using the compat API. You are potentially > breaking future distribution kernels by reverting to an obsolete and > deprecated API. > > You'll have to find a different way to do what you need. Eric described > pretty nicely a way to achieve that and how the fallbacks work, please > re-read his emails and modify the code accordingly. > > > The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when > > ERSPAN becomes > > fully supported. Going forward the ERSPAN feature is the determinant > > for whether gr/ip_gre > > compat mode is used or not. > > And with the next added feature to the kernel, that next feature will be > what determines whether the compat mode will be used? And then next and > so on? This doesn't work. ERSPAN must not be the decision factor. > Instead, rtnetlink must be tried first and if and only if it fails, > compat mode can be used. > > Please go read what Eric described about reading the value back. > > As for the patch, > > Nacked-by: Jiri Benc These patches have been reverted now, see https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/347916.html Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Fri, Jun 01, 2018 at 11:23:12AM -0700, William Tu wrote: > On Fri, Jun 1, 2018 at 9:38 AM, Jiri Benc wrote: > > On Fri, 1 Jun 2018 09:15:33 -0700, Gregory Rose wrote: > >> Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the > >> compat interface is needed > >> for kernels up to 4.15 so that we can support ERSPAN. If the built-in > >> gre/ip_gre kernel modules > >> don't have the ERSPAN support in them then we have to use the compat > >> interface. > > > > That's very wrong. The compat interface should not be used with > > upstream kernel (except perhaps for very very very old kernels). We > > converted the API to the standard rtnetlink for good reasons. New > > features are not supported using the compat API. You are potentially > > breaking future distribution kernels by reverting to an obsolete and > > deprecated API. > > > > You'll have to find a different way to do what you need. Eric described > > pretty nicely a way to achieve that and how the fallbacks work, please > > re-read his emails and modify the code accordingly. > > > >> The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when > >> ERSPAN becomes > >> fully supported. Going forward the ERSPAN feature is the determinant > >> for whether gr/ip_gre > >> compat mode is used or not. > > > > And with the next added feature to the kernel, that next feature will be > > what determines whether the compat mode will be used? And then next and > > so on? This doesn't work. ERSPAN must not be the decision factor. > > Instead, rtnetlink must be tried first and if and only if it fails, > > compat mode can be used. > > > > Please go read what Eric described about reading the value back. > > > Thanks for the feedback. > > Looking at the dpif_netlink_rtnl_probe_oot_tunnels(), since now we > added ERSPAN feature, instead of probing geneve module, > we should probe ip_gre module with a nlattr of ERSPAN (ex: HWID). > If it does not return -ENOSUPPORT, then use the upstream ip_gre module. > > And the next added feature should change this function to determine > whether to use compat mode or not. Do I understand it right? If we're going to continue adding new features to the out-of-tree (OVS shipped) modules then instead of using the compat interface by default the rtnetlink interface should always be tried first - with a fallback to compat (this catches things like LISP/STT). New features in the out-of-tree modules are then exposed via rtnetlink just like the in-tree modules. Code in lib/dpif-netlink-rtnl.c, vport_type_to_kind(), would need to be modified to try the "ovs_" prefix kinds. dpif_netlink_rtnl_probe_oot_tunnels() would the trigger for trying the "ovs_" prefixed kinds. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Fri, Jun 1, 2018 at 9:38 AM, Jiri Benc wrote: > On Fri, 1 Jun 2018 09:15:33 -0700, Gregory Rose wrote: >> Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the >> compat interface is needed >> for kernels up to 4.15 so that we can support ERSPAN. If the built-in >> gre/ip_gre kernel modules >> don't have the ERSPAN support in them then we have to use the compat >> interface. > > That's very wrong. The compat interface should not be used with > upstream kernel (except perhaps for very very very old kernels). We > converted the API to the standard rtnetlink for good reasons. New > features are not supported using the compat API. You are potentially > breaking future distribution kernels by reverting to an obsolete and > deprecated API. > > You'll have to find a different way to do what you need. Eric described > pretty nicely a way to achieve that and how the fallbacks work, please > re-read his emails and modify the code accordingly. > >> The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when >> ERSPAN becomes >> fully supported. Going forward the ERSPAN feature is the determinant >> for whether gr/ip_gre >> compat mode is used or not. > > And with the next added feature to the kernel, that next feature will be > what determines whether the compat mode will be used? And then next and > so on? This doesn't work. ERSPAN must not be the decision factor. > Instead, rtnetlink must be tried first and if and only if it fails, > compat mode can be used. > > Please go read what Eric described about reading the value back. > Thanks for the feedback. Looking at the dpif_netlink_rtnl_probe_oot_tunnels(), since now we added ERSPAN feature, instead of probing geneve module, we should probe ip_gre module with a nlattr of ERSPAN (ex: HWID). If it does not return -ENOSUPPORT, then use the upstream ip_gre module. And the next added feature should change this function to determine whether to use compat mode or not. Do I understand it right? Regards, William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Fri, Jun 1, 2018 at 9:56 AM, Gregory Rose wrote: > On 6/1/2018 9:38 AM, Jiri Benc wrote: >> >> On Fri, 1 Jun 2018 09:15:33 -0700, Gregory Rose wrote: >>> >>> Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the >>> compat interface is needed >>> for kernels up to 4.15 so that we can support ERSPAN. If the built-in >>> gre/ip_gre kernel modules >>> don't have the ERSPAN support in them then we have to use the compat >>> interface. >> >> That's very wrong. The compat interface should not be used with >> upstream kernel (except perhaps for very very very old kernels). We >> converted the API to the standard rtnetlink for good reasons. New >> features are not supported using the compat API. You are potentially >> breaking future distribution kernels by reverting to an obsolete and >> deprecated API. > > > Jiri, > > I think there must be some confusion here. The compat interface should be > used to support > features that are not in the kernel. ERSPAN in this case. ERSPAN is > introduced in 4.16 (let's > just agree to ignore 4.15 since it's not LTS). > > So in kernels up to and including 4.14 we must use the compat interface to > get ERSPAN support > correct? > > If there is another way to do it I'm all ears. > IIUC, I think Jiri and Eric consider the case where there is already a backport of ERSPAN in the order kernel, ex: RedHat backport ERSPAN to the older kernel. As a result, OVS needs to use the rtnetlink interface mentioned by Eric to probe the older kernel. If it has ERSPAN, then use the upstream module. Most of the cases for example ubuntu, it is a simple version check. If > 4.16 then use the upstream, otherwise use compat module. Regards, William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On 6/1/2018 9:38 AM, Jiri Benc wrote: On Fri, 1 Jun 2018 09:15:33 -0700, Gregory Rose wrote: Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the compat interface is needed for kernels up to 4.15 so that we can support ERSPAN. If the built-in gre/ip_gre kernel modules don't have the ERSPAN support in them then we have to use the compat interface. That's very wrong. The compat interface should not be used with upstream kernel (except perhaps for very very very old kernels). We converted the API to the standard rtnetlink for good reasons. New features are not supported using the compat API. You are potentially breaking future distribution kernels by reverting to an obsolete and deprecated API. You'll have to find a different way to do what you need. Eric described pretty nicely a way to achieve that and how the fallbacks work, please re-read his emails and modify the code accordingly. The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when ERSPAN becomes fully supported. Going forward the ERSPAN feature is the determinant for whether gr/ip_gre compat mode is used or not. And with the next added feature to the kernel, that next feature will be what determines whether the compat mode will be used? And then next and so on? This doesn't work. ERSPAN must not be the decision factor. Instead, rtnetlink must be tried first and if and only if it fails, compat mode can be used. Please go read what Eric described about reading the value back. As for the patch, Nacked-by: Jiri Benc Jiri Oh, I'm fine with reverting this patch. I think we were a bit hasty in getting it committed. Ben, could you please do so? thanks, - Greg ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On 6/1/2018 9:38 AM, Jiri Benc wrote: On Fri, 1 Jun 2018 09:15:33 -0700, Gregory Rose wrote: Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the compat interface is needed for kernels up to 4.15 so that we can support ERSPAN. If the built-in gre/ip_gre kernel modules don't have the ERSPAN support in them then we have to use the compat interface. That's very wrong. The compat interface should not be used with upstream kernel (except perhaps for very very very old kernels). We converted the API to the standard rtnetlink for good reasons. New features are not supported using the compat API. You are potentially breaking future distribution kernels by reverting to an obsolete and deprecated API. Jiri, I think there must be some confusion here. The compat interface should be used to support features that are not in the kernel. ERSPAN in this case. ERSPAN is introduced in 4.16 (let's just agree to ignore 4.15 since it's not LTS). So in kernels up to and including 4.14 we must use the compat interface to get ERSPAN support correct? If there is another way to do it I'm all ears. You'll have to find a different way to do what you need. Eric described pretty nicely a way to achieve that and how the fallbacks work, please re-read his emails and modify the code accordingly. The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when ERSPAN becomes fully supported. Going forward the ERSPAN feature is the determinant for whether gr/ip_gre compat mode is used or not. And with the next added feature to the kernel, that next feature will be what determines whether the compat mode will be used? And then next and so on? This doesn't work. ERSPAN must not be the decision factor. Instead, rtnetlink must be tried first and if and only if it fails, compat mode can be used. So how do we support ERSPAN in kernels that don't have it? I thought that's what the compat layer code is there for. Thanks, - Greg Please go read what Eric described about reading the value back. As for the patch, Nacked-by: Jiri Benc Jiri ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Fri, 1 Jun 2018 09:15:33 -0700, Gregory Rose wrote: > Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the > compat interface is needed > for kernels up to 4.15 so that we can support ERSPAN. If the built-in > gre/ip_gre kernel modules > don't have the ERSPAN support in them then we have to use the compat > interface. That's very wrong. The compat interface should not be used with upstream kernel (except perhaps for very very very old kernels). We converted the API to the standard rtnetlink for good reasons. New features are not supported using the compat API. You are potentially breaking future distribution kernels by reverting to an obsolete and deprecated API. You'll have to find a different way to do what you need. Eric described pretty nicely a way to achieve that and how the fallbacks work, please re-read his emails and modify the code accordingly. > The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when > ERSPAN becomes > fully supported. Going forward the ERSPAN feature is the determinant > for whether gr/ip_gre > compat mode is used or not. And with the next added feature to the kernel, that next feature will be what determines whether the compat mode will be used? And then next and so on? This doesn't work. ERSPAN must not be the decision factor. Instead, rtnetlink must be tried first and if and only if it fails, compat mode can be used. Please go read what Eric described about reading the value back. As for the patch, Nacked-by: Jiri Benc Jiri ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On 6/1/2018 8:30 AM, Eric Garver wrote:
On Fri, Jun 01, 2018 at 07:40:49AM -0700, Gregory Rose wrote:
On 6/1/2018 6:15 AM, Eric Garver wrote:
I'm a bit late, but I have comments below.
I'm also a bit out of touch, so I may be missing some context - if so, I
apologize.
On Thu, May 31, 2018 at 03:50:31PM -0700, Greg Rose wrote:
When verifying the built-in gre kernel module check for ERSPAN support.
Reported-by: Guru Shetty
Signed-off-by: Greg Rose
---
lib/dpif-netlink-rtnl.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index bec3fce..197cfb6 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -45,8 +45,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
#ifndef IFLA_GRE_MAX
#define IFLA_GRE_MAX 0
#endif
-#if IFLA_GRE_MAX < 18
-#define IFLA_GRE_COLLECT_METADATA 18
+#if IFLA_GRE_MAX < 24
+#define IFLA_GRE_ERSPAN_HWID 24
#endif
#ifndef IFLA_GENEVE_MAX
@@ -74,7 +74,7 @@ static const struct nl_policy vxlan_policy[] = {
[IFLA_VXLAN_GPE] = { .type = NL_A_FLAG, .optional = true },
};
static const struct nl_policy gre_policy[] = {
-[IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG },
I think we still need to verify _COLLECT_METADATA was passed back.
+[IFLA_GRE_ERSPAN_HWID] = { .type = NL_A_U16 },
};
static const struct nl_policy geneve_policy[] = {
[IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG },
@@ -207,7 +207,7 @@ dpif_netlink_rtnl_gre_verify(const struct
netdev_tunnel_config OVS_UNUSED *tnl,
err = rtnl_policy_parse(kind, reply, gre_policy, gre,
ARRAY_SIZE(gre_policy));
if (!err) {
-if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
+if (!nl_attr_get_u16(gre[IFLA_GRE_ERSPAN_HWID])) {
We need to verify COLLECT_METADATA is actually in use here. We should
only be checking for ERSPAN if ERSPAN was requested by the user.
I'm not super familiar with the code but the intent is to prevent the
built-in ip_gre kernel module
from being used if it doesn't support ERSPAN. If the built-in gre and
dpif_netlink_rtnl_probe_oot_tunnels() is responsible for deciding if
in-tree (linux) or out-of-tree (ovs) modules should be used.
There are two cases:
1) If the out-of-tree tunnel modules are loaded, the out-of-tree
compat interface will be used
2) otherwise in-tree tunnel modules are used (rtnetlink, i.e. code
in this file).
- if device create fails, then it falls back to the compat
interface
Since ERSPAN over gre/ip_gre was added to the Linux 4.16 kernel the
compat interface is needed
for kernels up to 4.15 so that we can support ERSPAN. If the built-in
gre/ip_gre kernel modules
don't have the ERSPAN support in them then we have to use the compat
interface.
ip_gre kernel modules
are loaded and the kernel doesn't have ERSPAN (4.16 and above) then OVS
users won't
be able to use ERSPAN features.
IIRC, the out-of-tree version of ip_gre will not be used on newer
kernels anyway. See references to USE_UPSTREAM_TUNNEL.
The target for USE_UPSTREAM_TUNNEL is moved to 4.16 now. That's when
ERSPAN becomes
fully supported. Going forward the ERSPAN feature is the determinant
for whether gr/ip_gre
compat mode is used or not.
Thanks,
- Greg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Fri, Jun 01, 2018 at 07:40:49AM -0700, Gregory Rose wrote:
> On 6/1/2018 6:15 AM, Eric Garver wrote:
> > I'm a bit late, but I have comments below.
> >
> > I'm also a bit out of touch, so I may be missing some context - if so, I
> > apologize.
> >
> > On Thu, May 31, 2018 at 03:50:31PM -0700, Greg Rose wrote:
> > > When verifying the built-in gre kernel module check for ERSPAN support.
> > >
> > > Reported-by: Guru Shetty
> > > Signed-off-by: Greg Rose
> > > ---
> > > lib/dpif-netlink-rtnl.c | 10 +-
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> > > index bec3fce..197cfb6 100644
> > > --- a/lib/dpif-netlink-rtnl.c
> > > +++ b/lib/dpif-netlink-rtnl.c
> > > @@ -45,8 +45,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
> > > #ifndef IFLA_GRE_MAX
> > > #define IFLA_GRE_MAX 0
> > > #endif
> > > -#if IFLA_GRE_MAX < 18
> > > -#define IFLA_GRE_COLLECT_METADATA 18
> > > +#if IFLA_GRE_MAX < 24
> > > +#define IFLA_GRE_ERSPAN_HWID 24
> > > #endif
> > > #ifndef IFLA_GENEVE_MAX
> > > @@ -74,7 +74,7 @@ static const struct nl_policy vxlan_policy[] = {
> > > [IFLA_VXLAN_GPE] = { .type = NL_A_FLAG, .optional = true },
> > > };
> > > static const struct nl_policy gre_policy[] = {
> > > -[IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG },
> > I think we still need to verify _COLLECT_METADATA was passed back.
> >
> > > +[IFLA_GRE_ERSPAN_HWID] = { .type = NL_A_U16 },
> > > };
> > > static const struct nl_policy geneve_policy[] = {
> > > [IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG },
> > > @@ -207,7 +207,7 @@ dpif_netlink_rtnl_gre_verify(const struct
> > > netdev_tunnel_config OVS_UNUSED *tnl,
> > > err = rtnl_policy_parse(kind, reply, gre_policy, gre,
> > > ARRAY_SIZE(gre_policy));
> > > if (!err) {
> > > -if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
> > > +if (!nl_attr_get_u16(gre[IFLA_GRE_ERSPAN_HWID])) {
> > We need to verify COLLECT_METADATA is actually in use here. We should
> > only be checking for ERSPAN if ERSPAN was requested by the user.
>
> I'm not super familiar with the code but the intent is to prevent the
> built-in ip_gre kernel module
> from being used if it doesn't support ERSPAN. If the built-in gre and
dpif_netlink_rtnl_probe_oot_tunnels() is responsible for deciding if
in-tree (linux) or out-of-tree (ovs) modules should be used.
There are two cases:
1) If the out-of-tree tunnel modules are loaded, the out-of-tree
compat interface will be used
2) otherwise in-tree tunnel modules are used (rtnetlink, i.e. code
in this file).
- if device create fails, then it falls back to the compat
interface
> ip_gre kernel modules
> are loaded and the kernel doesn't have ERSPAN (4.16 and above) then OVS
> users won't
> be able to use ERSPAN features.
IIRC, the out-of-tree version of ip_gre will not be used on newer
kernels anyway. See references to USE_UPSTREAM_TUNNEL.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On 6/1/2018 6:15 AM, Eric Garver wrote:
I'm a bit late, but I have comments below.
I'm also a bit out of touch, so I may be missing some context - if so, I
apologize.
On Thu, May 31, 2018 at 03:50:31PM -0700, Greg Rose wrote:
When verifying the built-in gre kernel module check for ERSPAN support.
Reported-by: Guru Shetty
Signed-off-by: Greg Rose
---
lib/dpif-netlink-rtnl.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index bec3fce..197cfb6 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -45,8 +45,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
#ifndef IFLA_GRE_MAX
#define IFLA_GRE_MAX 0
#endif
-#if IFLA_GRE_MAX < 18
-#define IFLA_GRE_COLLECT_METADATA 18
+#if IFLA_GRE_MAX < 24
+#define IFLA_GRE_ERSPAN_HWID 24
#endif
#ifndef IFLA_GENEVE_MAX
@@ -74,7 +74,7 @@ static const struct nl_policy vxlan_policy[] = {
[IFLA_VXLAN_GPE] = { .type = NL_A_FLAG, .optional = true },
};
static const struct nl_policy gre_policy[] = {
-[IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG },
I think we still need to verify _COLLECT_METADATA was passed back.
+[IFLA_GRE_ERSPAN_HWID] = { .type = NL_A_U16 },
};
static const struct nl_policy geneve_policy[] = {
[IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG },
@@ -207,7 +207,7 @@ dpif_netlink_rtnl_gre_verify(const struct
netdev_tunnel_config OVS_UNUSED *tnl,
err = rtnl_policy_parse(kind, reply, gre_policy, gre,
ARRAY_SIZE(gre_policy));
if (!err) {
-if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
+if (!nl_attr_get_u16(gre[IFLA_GRE_ERSPAN_HWID])) {
We need to verify COLLECT_METADATA is actually in use here. We should
only be checking for ERSPAN if ERSPAN was requested by the user.
I'm not super familiar with the code but the intent is to prevent the
built-in ip_gre kernel module
from being used if it doesn't support ERSPAN. If the built-in gre and
ip_gre kernel modules
are loaded and the kernel doesn't have ERSPAN (4.16 and above) then OVS
users won't
be able to use ERSPAN features.
- Greg
err = EINVAL;
}
}
@@ -328,7 +328,7 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config
*tnl_cfg,
case OVS_VPORT_TYPE_ERSPAN:
case OVS_VPORT_TYPE_IP6ERSPAN:
case OVS_VPORT_TYPE_IP6GRE:
-nl_msg_put_flag(, IFLA_GRE_COLLECT_METADATA);
I think we still need to use COLLECT_METADATA. Don't we depend on using
lwt?
+nl_msg_put_u16(, IFLA_GRE_ERSPAN_HWID, 0xdead);
break;
case OVS_VPORT_TYPE_GENEVE:
nl_msg_put_flag(, IFLA_GENEVE_COLLECT_METADATA);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
I'm a bit late, but I have comments below.
I'm also a bit out of touch, so I may be missing some context - if so, I
apologize.
On Thu, May 31, 2018 at 03:50:31PM -0700, Greg Rose wrote:
> When verifying the built-in gre kernel module check for ERSPAN support.
>
> Reported-by: Guru Shetty
> Signed-off-by: Greg Rose
> ---
> lib/dpif-netlink-rtnl.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index bec3fce..197cfb6 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -45,8 +45,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
> #ifndef IFLA_GRE_MAX
> #define IFLA_GRE_MAX 0
> #endif
> -#if IFLA_GRE_MAX < 18
> -#define IFLA_GRE_COLLECT_METADATA 18
> +#if IFLA_GRE_MAX < 24
> +#define IFLA_GRE_ERSPAN_HWID 24
> #endif
>
> #ifndef IFLA_GENEVE_MAX
> @@ -74,7 +74,7 @@ static const struct nl_policy vxlan_policy[] = {
> [IFLA_VXLAN_GPE] = { .type = NL_A_FLAG, .optional = true },
> };
> static const struct nl_policy gre_policy[] = {
> -[IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG },
I think we still need to verify _COLLECT_METADATA was passed back.
> +[IFLA_GRE_ERSPAN_HWID] = { .type = NL_A_U16 },
> };
> static const struct nl_policy geneve_policy[] = {
> [IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG },
> @@ -207,7 +207,7 @@ dpif_netlink_rtnl_gre_verify(const struct
> netdev_tunnel_config OVS_UNUSED *tnl,
> err = rtnl_policy_parse(kind, reply, gre_policy, gre,
> ARRAY_SIZE(gre_policy));
> if (!err) {
> -if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
> +if (!nl_attr_get_u16(gre[IFLA_GRE_ERSPAN_HWID])) {
We need to verify COLLECT_METADATA is actually in use here. We should
only be checking for ERSPAN if ERSPAN was requested by the user.
> err = EINVAL;
> }
> }
> @@ -328,7 +328,7 @@ dpif_netlink_rtnl_create(const struct
> netdev_tunnel_config *tnl_cfg,
> case OVS_VPORT_TYPE_ERSPAN:
> case OVS_VPORT_TYPE_IP6ERSPAN:
> case OVS_VPORT_TYPE_IP6GRE:
> -nl_msg_put_flag(, IFLA_GRE_COLLECT_METADATA);
I think we still need to use COLLECT_METADATA. Don't we depend on using
lwt?
> +nl_msg_put_u16(, IFLA_GRE_ERSPAN_HWID, 0xdead);
> break;
> case OVS_VPORT_TYPE_GENEVE:
> nl_msg_put_flag(, IFLA_GENEVE_COLLECT_METADATA);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
On Thu, May 31, 2018 at 03:50:31PM -0700, Greg Rose wrote: > When verifying the built-in gre kernel module check for ERSPAN support. > > Reported-by: Guru Shetty > Signed-off-by: Greg Rose Thanks, I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
When verifying the built-in gre kernel module check for ERSPAN support.
Reported-by: Guru Shetty
Signed-off-by: Greg Rose
---
lib/dpif-netlink-rtnl.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index bec3fce..197cfb6 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -45,8 +45,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
#ifndef IFLA_GRE_MAX
#define IFLA_GRE_MAX 0
#endif
-#if IFLA_GRE_MAX < 18
-#define IFLA_GRE_COLLECT_METADATA 18
+#if IFLA_GRE_MAX < 24
+#define IFLA_GRE_ERSPAN_HWID 24
#endif
#ifndef IFLA_GENEVE_MAX
@@ -74,7 +74,7 @@ static const struct nl_policy vxlan_policy[] = {
[IFLA_VXLAN_GPE] = { .type = NL_A_FLAG, .optional = true },
};
static const struct nl_policy gre_policy[] = {
-[IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG },
+[IFLA_GRE_ERSPAN_HWID] = { .type = NL_A_U16 },
};
static const struct nl_policy geneve_policy[] = {
[IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG },
@@ -207,7 +207,7 @@ dpif_netlink_rtnl_gre_verify(const struct
netdev_tunnel_config OVS_UNUSED *tnl,
err = rtnl_policy_parse(kind, reply, gre_policy, gre,
ARRAY_SIZE(gre_policy));
if (!err) {
-if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
+if (!nl_attr_get_u16(gre[IFLA_GRE_ERSPAN_HWID])) {
err = EINVAL;
}
}
@@ -328,7 +328,7 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config
*tnl_cfg,
case OVS_VPORT_TYPE_ERSPAN:
case OVS_VPORT_TYPE_IP6ERSPAN:
case OVS_VPORT_TYPE_IP6GRE:
-nl_msg_put_flag(, IFLA_GRE_COLLECT_METADATA);
+nl_msg_put_u16(, IFLA_GRE_ERSPAN_HWID, 0xdead);
break;
case OVS_VPORT_TYPE_GENEVE:
nl_msg_put_flag(, IFLA_GENEVE_COLLECT_METADATA);
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
