Re: [ovs-dev] [PATCH V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

[Gregory Rose]

On 1/23/2018 1:07 PM, Justin Pettit wrote:

On Jan 23, 2018, at 12:01 PM, Gregory Rose  wrote:

On 1/23/2018 12:00 PM, Justin Pettit wrote:

On Jan 5, 2018, at 11:30 AM, Greg Rose  wrote:

+#ifdef frag_percpu_counter_batch
...
+#else /* frag_percpu_counter_batch */

This is kind of a nit, but I would have thought this "#else" comment would be 
"!frag_percpu_counter_batch", since that's the case when it's not defined.  However, I'm 
not sure how it's handled usually in the kernel.  Looking through our compat code, I see examples 
of it being done both ways, though.  Thoughts?

--Justin



It's a valid nit.

I can send V2 or you can fix it on push.  Which do you prefer?

I went ahead and made the change.  I pushed it to master and branch-2.9.

--Justin




Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

[Justin Pettit]

> On Jan 23, 2018, at 12:34 PM, Ben Pfaff  wrote:
> 
> On Tue, Jan 23, 2018 at 12:00:20PM -0800, Justin Pettit wrote:
>> 
>> 
>>> On Jan 5, 2018, at 11:30 AM, Greg Rose  wrote:
>>> 
>>> +#ifdef frag_percpu_counter_batch
>>> ...
>>> +#else /* frag_percpu_counter_batch */
>> 
>> This is kind of a nit, but I would have thought this "#else" comment
>> would be "!frag_percpu_counter_batch", since that's the case when it's
>> not defined.  However, I'm not sure how it's handled usually in the
>> kernel.  Looking through our compat code, I see examples of it being
>> done both ways, though.  Thoughts?
> 
> This is one of those weird places where style differs.  GNU coding
> standards would mandate "!frag_percpu_counter_batch" here, and that's
> what I prefer myself, but I've seen the opposite convention in (if I
> recall correctly) some BSD code.

Interesting.  I decided to add the "!", since I think it makes it clearer.

--Justin


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

[Justin Pettit]

> On Jan 23, 2018, at 12:01 PM, Gregory Rose  wrote:
> 
> On 1/23/2018 12:00 PM, Justin Pettit wrote:
>> 
>>> On Jan 5, 2018, at 11:30 AM, Greg Rose  wrote:
>>> 
>>> +#ifdef frag_percpu_counter_batch
>>> ...
>>> +#else /* frag_percpu_counter_batch */
>> This is kind of a nit, but I would have thought this "#else" comment would 
>> be "!frag_percpu_counter_batch", since that's the case when it's not 
>> defined.  However, I'm not sure how it's handled usually in the kernel.  
>> Looking through our compat code, I see examples of it being done both ways, 
>> though.  Thoughts?
>> 
>> --Justin
>> 
>> 
> It's a valid nit.
> 
> I can send V2 or you can fix it on push.  Which do you prefer?

I went ahead and made the change.  I pushed it to master and branch-2.9.

--Justin



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

[Ben Pfaff]

On Tue, Jan 23, 2018 at 12:00:20PM -0800, Justin Pettit wrote:
> 
> 
> > On Jan 5, 2018, at 11:30 AM, Greg Rose  wrote:
> > 
> > +#ifdef frag_percpu_counter_batch
> > ...
> > +#else /* frag_percpu_counter_batch */
> 
> This is kind of a nit, but I would have thought this "#else" comment
> would be "!frag_percpu_counter_batch", since that's the case when it's
> not defined.  However, I'm not sure how it's handled usually in the
> kernel.  Looking through our compat code, I see examples of it being
> done both ways, though.  Thoughts?

This is one of those weird places where style differs.  GNU coding
standards would mandate "!frag_percpu_counter_batch" here, and that's
what I prefer myself, but I've seen the opposite convention in (if I
recall correctly) some BSD code.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

[Gregory Rose]

On 1/23/2018 12:00 PM, Justin Pettit wrote:



On Jan 5, 2018, at 11:30 AM, Greg Rose  wrote:

+#ifdef frag_percpu_counter_batch
...
+#else /* frag_percpu_counter_batch */

This is kind of a nit, but I would have thought this "#else" comment would be 
"!frag_percpu_counter_batch", since that's the case when it's not defined.  However, I'm 
not sure how it's handled usually in the kernel.  Looking through our compat code, I see examples 
of it being done both ways, though.  Thoughts?

--Justin



It's a valid nit.

I can send V2 or you can fix it on push.  Which do you prefer?

Thanks,

- Greg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

[Justin Pettit]

> On Jan 5, 2018, at 11:30 AM, Greg Rose  wrote:
> 
> +#ifdef frag_percpu_counter_batch
> ...
> +#else /* frag_percpu_counter_batch */

This is kind of a nit, but I would have thought this "#else" comment would be 
"!frag_percpu_counter_batch", since that's the case when it's not defined.  
However, I'm not sure how it's handled usually in the kernel.  Looking through 
our compat code, I see examples of it being done both ways, though.  Thoughts?

--Justin


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

[Greg Rose]

Fix up the compat layer to check for frag_percpu_counter_batch and
if not present then use atomic_sub and atomic_add as per the
backport in the 3.16.50 LTS kernel.  Fixes compile errors on
3.16 series kernels from 3.16.50 on.

Signed-off-by: Greg Rose 

---

V2 - Fix typo
---
 datapath/linux/compat/include/net/inet_frag.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/datapath/linux/compat/include/net/inet_frag.h 
b/datapath/linux/compat/include/net/inet_frag.h
index 34078c8..c98c3a4 100644
--- a/datapath/linux/compat/include/net/inet_frag.h
+++ b/datapath/linux/compat/include/net/inet_frag.h
@@ -30,6 +30,7 @@ static inline bool inet_frag_evicting(struct inet_frag_queue 
*q)
 #endif
 
 #ifndef HAVE_SUB_FRAG_MEM_LIMIT_ARG_STRUCT_NETNS_FRAGS
+#ifdef frag_percpu_counter_batch
 static inline void rpl_sub_frag_mem_limit(struct netns_frags *nf, int i)
 {
__percpu_counter_add(>mem, -i, frag_percpu_counter_batch);
@@ -41,6 +42,19 @@ static inline void rpl_add_frag_mem_limit(struct netns_frags 
*nf, int i)
__percpu_counter_add(>mem, i, frag_percpu_counter_batch);
 }
 #define add_frag_mem_limit rpl_add_frag_mem_limit
+#else /* frag_percpu_counter_batch */
+static inline void rpl_sub_frag_mem_limit(struct netns_frags *nf, int i)
+{
+   atomic_sub(i, >mem);
+}
+#define sub_frag_mem_limit rpl_sub_frag_mem_limit
+
+static inline void rpl_add_frag_mem_limit(struct netns_frags *nf, int i)
+{
+   atomic_add(i, >mem);
+}
+#define add_frag_mem_limit rpl_add_frag_mem_limit
+#endif /* frag_percpu_counter_batch */
 #endif
 
 #ifdef HAVE_VOID_INET_FRAGS_INIT
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev