[ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support
Gregory Rose
gvrose8192 at gmail.com
Fri Jun 1 16:15:33 UTC 2018
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 <guru at ovn.org>
>>>> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
>>>> ---
>>>> 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
More information about the dev
mailing list