On Tue, Jul 10, 2018 at 4:27 PM, Yifeng Sun <pkusunyif...@gmail.com> wrote:
> Looks good to me, thanks.
>
> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com>
>
>
>
> On Tue, Jul 10, 2018 at 10:50 AM, William Tu <u9012...@gmail.com> wrote:
>>
>> Commit 9f57c67c379d ("gre: Remove support for sharing GRE protocol hook")
>> allows only single GRE packet receiver.  When upstream kernel's gre module
>> is loaded, the gre.ko exclusively becomes the only gre packet receiver,
>> preventing OVS kernel module from registering another gre receiver.
>>
>> We can either try to unload the gre.ko by removing its dependencies,
>> or, in this patch, we try to register OVS as only the GRE transmit
>> portion when detecting there already exists another GRE receiver.
>>
>> Signed-off-by: William Tu <u9012...@gmail.com>
>> Cc: Greg Rose <gvrose8...@gmail.com>
>> Cc: Yifeng Sun <pkusunyif...@gmail.com>
>> ---
>>  datapath/linux/compat/ip_gre.c | 60
>> +++++++++++++++++++++++++++++++++---------
>>  datapath/vport.c               | 12 ++++++---
>>  2 files changed, 56 insertions(+), 16 deletions(-)
>>
>> diff --git a/datapath/linux/compat/ip_gre.c
>> b/datapath/linux/compat/ip_gre.c
>> index 92de70127189..1ab798164894 100644
>> --- a/datapath/linux/compat/ip_gre.c
>> +++ b/datapath/linux/compat/ip_gre.c
>> @@ -71,6 +71,7 @@ static void erspan_build_header(struct sk_buff *skb,
>>                                 bool truncate, bool is_ipv4);
>>
>>  static struct rtnl_link_ops ipgre_link_ops __read_mostly;
>> +static bool ip_gre_loaded = false;
>>
>>  #define ip_gre_calc_hlen rpl_ip_gre_calc_hlen
>>  static int ip_gre_calc_hlen(__be16 o_flags)
>> @@ -1640,25 +1641,57 @@ int rpl_ipgre_init(void)
>>         int err;
>>
>>         err = register_pernet_device(&ipgre_tap_net_ops);
>> -       if (err < 0)
>> -               goto pnet_tap_failed;
>> +       if (err < 0) {
>> +               if (err == -EEXIST)
>> +                       goto ip_gre_loaded;
>> +               else
>> +                       goto pnet_tap_failed;
>> +       }
>>
>>         err = register_pernet_device(&erspan_net_ops);
>> -       if (err < 0)
>> -               goto pnet_erspan_failed;
>> +       if (err < 0) {
>> +               if (err == -EEXIST)
>> +                       goto ip_gre_loaded;
>> +               else
>> +                       goto pnet_erspan_failed;
>> +       }
>>
>>         err = register_pernet_device(&ipgre_net_ops);
>> -       if (err < 0)
>> -               goto pnet_ipgre_failed;
>> +       if (err < 0) {
>> +               if (err == -EEXIST)
>> +                       goto ip_gre_loaded;
>> +               else
>> +                       goto pnet_ipgre_failed;
>> +       }
>>
>>         err = gre_add_protocol(&ipgre_protocol, GREPROTO_CISCO);
>>         if (err < 0) {
>>                 pr_info("%s: can't add protocol\n", __func__);
>> -               goto add_proto_failed;
>> +               if (err == -EBUSY) {
>> +                       goto ip_gre_loaded;
>> +               } else {
>> +                       goto add_proto_failed;
>> +               }
>>         }
>>
>>         pr_info("GRE over IPv4 tunneling driver\n");
>> -
>> +       ovs_vport_ops_register(&ovs_ipgre_vport_ops);
>> +       ovs_vport_ops_register(&ovs_erspan_vport_ops);
>> +       return 0;
>> +
>> +ip_gre_loaded:
>> +       /* Since GRE only allows single receiver to be registerd,
>> +        * we skip here so only gre transmit works, see:
>> +        *
>> +        * commit 9f57c67c379d88a10e8ad676426fee5ae7341b14
>> +        * Author: Pravin B Shelar <pshe...@nicira.com>
>> +        * Date:   Fri Aug 7 23:51:52 2015 -0700
>> +        *     gre: Remove support for sharing GRE protocol hook
>> +        *
>> +        * OVS GRE receive part is disabled.
>> +        */
>> +       pr_info("GRE TX only over IPv4 tunneling driver\n");
>> +       ip_gre_loaded = true;
>>         ovs_vport_ops_register(&ovs_ipgre_vport_ops);
>>         ovs_vport_ops_register(&ovs_erspan_vport_ops);
>>         return 0;
>> @@ -1678,10 +1711,13 @@ void rpl_ipgre_fini(void)
>>  {
>>         ovs_vport_ops_unregister(&ovs_erspan_vport_ops);
>>         ovs_vport_ops_unregister(&ovs_ipgre_vport_ops);
>> -       gre_del_protocol(&ipgre_protocol, GREPROTO_CISCO);
>> -       unregister_pernet_device(&ipgre_net_ops);
>> -       unregister_pernet_device(&erspan_net_ops);
>> -       unregister_pernet_device(&ipgre_tap_net_ops);
>> +
>> +       if (!ip_gre_loaded) {
>> +               gre_del_protocol(&ipgre_protocol, GREPROTO_CISCO);
>> +               unregister_pernet_device(&ipgre_net_ops);
>> +               unregister_pernet_device(&erspan_net_ops);
>> +               unregister_pernet_device(&ipgre_tap_net_ops);
>> +       }
>>  }
>>
>>  #endif
>> diff --git a/datapath/vport.c b/datapath/vport.c
>> index 02f6b56d3243..05dd9dc2a149 100644
>> --- a/datapath/vport.c
>> +++ b/datapath/vport.c
>> @@ -66,11 +66,15 @@ int ovs_vport_init(void)
>>         if (err)
>>                 goto err_lisp;
>>         err = gre_init();
>> -       if (err && err != -EEXIST)
>> +       if (err && err != -EEXIST) {
>>                 goto err_gre;
>> -       else if (err == -EEXIST)
>> -               pr_warn("Cannot take GRE protocol entry - The ERSPAN
>> feature may not be supported\n");
>> -       else {
>> +       } else {
>> +               if (err == -EEXIST) {
>> +                       pr_warn("Cannot take GRE protocol entry"\
>> +                               "- The ERSPAN feature may not be
>> supported\n");


thanks for the review. I found the above warning message a bit confusing.
In the case above, only the RX part of GRE/ERSPAN is not supported.
I will resubmit v2.

Regards,
William
>> +                       /* continue GRE tx */
>> +               }
>> +
>>                 err = ipgre_init();
>>                 if (err && err != -EEXIST)
>>                         goto err_ipgre;
>> --
>> 2.7.4
>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to