Right, that's a cleaner solution.  Thanks for pointing it out.  I
think there are a few others like that in the series then.

- Greg

On Fri, Jul 21, 2017 at 11:42 AM, Joe Stringer <[email protected]> wrote:
> On 19 July 2017 at 13:57, Greg Rose <[email protected]> wrote:
>> Upstream commit:
>>     commit d91fc59cd77c719f33eda65c194ad8f95a055190
>>     Author: Liping Zhang <[email protected]>
>>     Date:   Sun May 7 22:01:55 2017 +0800
>>
>>     netfilter: introduce nf_conntrack_helper_put helper function
>>
>>     And convert module_put invocation to nf_conntrack_helper_put, this is
>>     prepared for the followup patch, which will add a refcnt for cthelper,
>>     so we can reject the deleting request when cthelper is in use.
>>
>>     Signed-off-by: Liping Zhang <[email protected]>
>>     Signed-off-by: Pablo Neira Ayuso <[email protected]>
>>
>> Applied with additional use of HAVE_NF_CONNTRACK_HELPER_PUT compatibility
>> flag defined in acinclude.m4.
>>
>> Signed-off-by: Greg Rose <[email protected]>
>> ---
>>  datapath/conntrack.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
>> index bf28fc0..cce2642 100644
>> --- a/datapath/conntrack.c
>> +++ b/datapath/conntrack.c
>> @@ -1164,7 +1164,11 @@ static int ovs_ct_add_helper(struct 
>> ovs_conntrack_info *info, const char *name,
>>
>>         help = nf_ct_helper_ext_add(info->ct, helper, GFP_KERNEL);
>>         if (!help) {
>> +#ifdef HAVE_NF_CONTRACK_HELPER_PUT
>> +               nf_contrack_helper_put(helper);
>> +#else
>>                 module_put(helper->me);
>> +#endif
>>                 return -ENOMEM;
>>         }
>>
>> @@ -1634,7 +1638,11 @@ void ovs_ct_free_action(const struct nlattr *a)
>>  static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info)
>>  {
>>         if (ct_info->helper)
>> +#ifdef HAVE_NF_CONNTRACK_HELPER_PUT
>> +               nf_conntrack_helper_put(ct_info->helper);
>> +#else
>>                 module_put(ct_info->helper->me);
>> +#endif
>>         if (ct_info->ct)
>>                 nf_ct_tmpl_free(ct_info->ct);
>>  }
>
> Ideally the datapath/*.[ch] code is as close to upstream as possible,
> then the datapath/linux/compat/* takes care of making this happen.
>
> To achieve this, in this kind of case, what we will often do is to
> introduce a new function like this in the
> datapath/linux/compat/net/netfilter/nf_conntrack_helper.h:
>
> #ifndef HAVE_NF_CONNTRACK_HELPER_PUT
> static inline void nf_conntrack_helper_put(struct nf_conntrack_helper *helper)
> {
>     module_put(helper->me);
> }
> #endif
>
> Then, in datapath/conntrack.c we don't need any #ifdef; it can just
> directly call the new nf_conntrack_helper_put() function.
>
> Thanks.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to