Re: [PATCH] flow_dissector: Use 'const' where possible.
From: Jiri Pirko
Date: Wed, 2 Sep 2015 18:39:34 +0200
> Wed, Sep 02, 2015 at 06:33:34AM CEST, t...@herbertland.com wrote:
>>> @@ -19,14 +19,14 @@
>>> #include
>>> #include
>>>
>>> -static bool skb_flow_dissector_uses_key(struct flow_dissector
>>> *flow_dissector,
>>> - enum flow_dissector_key_id key_id)
>>> +static bool dissector_uses_key(const struct flow_dissector *flow_dissector,
>>> + enum flow_dissector_key_id key_id)
>>> {
>>> return flow_dissector->used_keys & (1 << key_id);
>>> }
>>>
>>> -static void skb_flow_dissector_set_key(struct flow_dissector
>>> *flow_dissector,
>>> - enum flow_dissector_key_id key_id)
>>> +static void dissector_set_key(struct flow_dissector *flow_dissector,
>>> + enum flow_dissector_key_id key_id)
>>> {
>>> flow_dissector->used_keys |= (1 << key_id);
>>> }
>>> @@ -51,20 +51,20 @@ void skb_flow_dissector_init(struct flow_dissector
>>> *flow_dissector,
>>
>>I suppose we should drop skb_ from skb_flow_dissector_init and
>>skb_flow_dissector_target as well.
>
> I like to have "namespaces" by function prefixes. Code is easier to read
> then...
I completely disagree.
These are static, local functions, the can use whatever names they want
and the shorter the better.
Long function names drive me absolutely insane and make keeping the
argument lists under ~80 columns a royal pain in the ass.
So I will continue to trim function names down to something more
reasonable when they are static and local to a source file.
And I encourage you to do so as well.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] flow_dissector: Use 'const' where possible.
On Wed, Sep 2, 2015 at 9:39 AM, Jiri Pirko wrote:
> Wed, Sep 02, 2015 at 06:33:34AM CEST, t...@herbertland.com wrote:
>>On Tue, Sep 1, 2015 at 9:19 PM, David Miller wrote:
>>>
>>> Signed-off-by: David S. Miller
>>> ---
>>> include/linux/skbuff.h| 8 ++---
>>> include/net/flow.h| 8 ++---
>>> net/core/flow_dissector.c | 79
>>> ---
>>> 3 files changed, 49 insertions(+), 46 deletions(-)
>>>
>
>
>
>
>>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>>> index 345a040..d79699c 100644
>>> --- a/net/core/flow_dissector.c
>>> +++ b/net/core/flow_dissector.c
>>> @@ -19,14 +19,14 @@
>>> #include
>>> #include
>>>
>>> -static bool skb_flow_dissector_uses_key(struct flow_dissector
>>> *flow_dissector,
>>> - enum flow_dissector_key_id key_id)
>>> +static bool dissector_uses_key(const struct flow_dissector *flow_dissector,
>>> + enum flow_dissector_key_id key_id)
>>> {
>>> return flow_dissector->used_keys & (1 << key_id);
>>> }
>>>
>>> -static void skb_flow_dissector_set_key(struct flow_dissector
>>> *flow_dissector,
>>> - enum flow_dissector_key_id key_id)
>>> +static void dissector_set_key(struct flow_dissector *flow_dissector,
>>> + enum flow_dissector_key_id key_id)
>>> {
>>> flow_dissector->used_keys |= (1 << key_id);
>>> }
>>> @@ -51,20 +51,20 @@ void skb_flow_dissector_init(struct flow_dissector
>>> *flow_dissector,
>>
>>I suppose we should drop skb_ from skb_flow_dissector_init and
>>skb_flow_dissector_target as well.
>
> I like to have "namespaces" by function prefixes. Code is easier to read
> then...
Right, these functions now are independent of sk_buff. Conceptually
someone could use these for a non-skbuff application-- so it's good
design!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] flow_dissector: Use 'const' where possible.
Wed, Sep 02, 2015 at 06:33:34AM CEST, t...@herbertland.com wrote:
>On Tue, Sep 1, 2015 at 9:19 PM, David Miller wrote:
>>
>> Signed-off-by: David S. Miller
>> ---
>> include/linux/skbuff.h| 8 ++---
>> include/net/flow.h| 8 ++---
>> net/core/flow_dissector.c | 79
>> ---
>> 3 files changed, 49 insertions(+), 46 deletions(-)
>>
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 345a040..d79699c 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -19,14 +19,14 @@
>> #include
>> #include
>>
>> -static bool skb_flow_dissector_uses_key(struct flow_dissector
>> *flow_dissector,
>> - enum flow_dissector_key_id key_id)
>> +static bool dissector_uses_key(const struct flow_dissector *flow_dissector,
>> + enum flow_dissector_key_id key_id)
>> {
>> return flow_dissector->used_keys & (1 << key_id);
>> }
>>
>> -static void skb_flow_dissector_set_key(struct flow_dissector
>> *flow_dissector,
>> - enum flow_dissector_key_id key_id)
>> +static void dissector_set_key(struct flow_dissector *flow_dissector,
>> + enum flow_dissector_key_id key_id)
>> {
>> flow_dissector->used_keys |= (1 << key_id);
>> }
>> @@ -51,20 +51,20 @@ void skb_flow_dissector_init(struct flow_dissector
>> *flow_dissector,
>
>I suppose we should drop skb_ from skb_flow_dissector_init and
>skb_flow_dissector_target as well.
I like to have "namespaces" by function prefixes. Code is easier to read
then...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] flow_dissector: Use 'const' where possible.
From: Tom Herbert
Date: Tue, 1 Sep 2015 21:33:34 -0700
>> @@ -19,14 +19,14 @@
>> #include
>> #include
>>
>> -static bool skb_flow_dissector_uses_key(struct flow_dissector
>> *flow_dissector,
>> - enum flow_dissector_key_id key_id)
>> +static bool dissector_uses_key(const struct flow_dissector *flow_dissector,
>> + enum flow_dissector_key_id key_id)
>> {
>> return flow_dissector->used_keys & (1 << key_id);
>> }
>>
>> -static void skb_flow_dissector_set_key(struct flow_dissector
>> *flow_dissector,
>> - enum flow_dissector_key_id key_id)
>> +static void dissector_set_key(struct flow_dissector *flow_dissector,
>> + enum flow_dissector_key_id key_id)
>> {
>> flow_dissector->used_keys |= (1 << key_id);
>> }
>> @@ -51,20 +51,20 @@ void skb_flow_dissector_init(struct flow_dissector
>> *flow_dissector,
>
> I suppose we should drop skb_ from skb_flow_dissector_init and
> skb_flow_dissector_target as well.
I did it here because the functions were local and the lines would be
too long for even the first argument otherwise.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] flow_dissector: Use 'const' where possible.
On Tue, 2015-09-01 at 21:19 -0700, David Miller wrote: > Signed-off-by: David S. Miller [] > net/core/flow_dissector.c | 79 > --- Thanks David. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] flow_dissector: Use 'const' where possible.
On Tue, Sep 1, 2015 at 9:19 PM, David Miller wrote:
>
> Signed-off-by: David S. Miller
> ---
> include/linux/skbuff.h| 8 ++---
> include/net/flow.h| 8 ++---
> net/core/flow_dissector.c | 79
> ---
> 3 files changed, 49 insertions(+), 46 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index eabfb81..2738d35 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1029,9 +1029,9 @@ static inline __u32 skb_get_hash(struct sk_buff *skb)
> return skb->hash;
> }
>
> -__u32 __skb_get_hash_flowi6(struct sk_buff *skb, struct flowi6 *fl6);
> +__u32 __skb_get_hash_flowi6(struct sk_buff *skb, const struct flowi6 *fl6);
>
> -static inline __u32 skb_get_hash_flowi6(struct sk_buff *skb, struct flowi6
> *fl6)
> +static inline __u32 skb_get_hash_flowi6(struct sk_buff *skb, const struct
> flowi6 *fl6)
> {
> if (!skb->l4_hash && !skb->sw_hash) {
> struct flow_keys keys;
> @@ -1043,9 +1043,9 @@ static inline __u32 skb_get_hash_flowi6(struct sk_buff
> *skb, struct flowi6 *fl6)
> return skb->hash;
> }
>
> -__u32 __skb_get_hash_flowi4(struct sk_buff *skb, struct flowi4 *fl);
> +__u32 __skb_get_hash_flowi4(struct sk_buff *skb, const struct flowi4 *fl);
>
> -static inline __u32 skb_get_hash_flowi4(struct sk_buff *skb, struct flowi4
> *fl4)
> +static inline __u32 skb_get_hash_flowi4(struct sk_buff *skb, const struct
> flowi4 *fl4)
> {
> if (!skb->l4_hash && !skb->sw_hash) {
> struct flow_keys keys;
> diff --git a/include/net/flow.h b/include/net/flow.h
> index dafe97c..acd6a09 100644
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
> @@ -244,18 +244,18 @@ void flow_cache_flush(struct net *net);
> void flow_cache_flush_deferred(struct net *net);
> extern atomic_t flow_cache_genid;
>
> -__u32 __get_hash_from_flowi6(struct flowi6 *fl6, struct flow_keys *keys);
> +__u32 __get_hash_from_flowi6(const struct flowi6 *fl6, struct flow_keys
> *keys);
>
> -static inline __u32 get_hash_from_flowi6(struct flowi6 *fl6)
> +static inline __u32 get_hash_from_flowi6(const struct flowi6 *fl6)
> {
> struct flow_keys keys;
>
> return __get_hash_from_flowi6(fl6, &keys);
> }
>
> -__u32 __get_hash_from_flowi4(struct flowi4 *fl4, struct flow_keys *keys);
> +__u32 __get_hash_from_flowi4(const struct flowi4 *fl4, struct flow_keys
> *keys);
>
> -static inline __u32 get_hash_from_flowi4(struct flowi4 *fl4)
> +static inline __u32 get_hash_from_flowi4(const struct flowi4 *fl4)
> {
> struct flow_keys keys;
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 345a040..d79699c 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -19,14 +19,14 @@
> #include
> #include
>
> -static bool skb_flow_dissector_uses_key(struct flow_dissector
> *flow_dissector,
> - enum flow_dissector_key_id key_id)
> +static bool dissector_uses_key(const struct flow_dissector *flow_dissector,
> + enum flow_dissector_key_id key_id)
> {
> return flow_dissector->used_keys & (1 << key_id);
> }
>
> -static void skb_flow_dissector_set_key(struct flow_dissector *flow_dissector,
> - enum flow_dissector_key_id key_id)
> +static void dissector_set_key(struct flow_dissector *flow_dissector,
> + enum flow_dissector_key_id key_id)
> {
> flow_dissector->used_keys |= (1 << key_id);
> }
> @@ -51,20 +51,20 @@ void skb_flow_dissector_init(struct flow_dissector
> *flow_dissector,
I suppose we should drop skb_ from skb_flow_dissector_init and
skb_flow_dissector_target as well.
> * boundaries of unsigned short.
> */
> BUG_ON(key->offset > USHRT_MAX);
> - BUG_ON(skb_flow_dissector_uses_key(flow_dissector,
> - key->key_id));
> + BUG_ON(dissector_uses_key(flow_dissector,
> + key->key_id));
>
> - skb_flow_dissector_set_key(flow_dissector, key->key_id);
> + dissector_set_key(flow_dissector, key->key_id);
> flow_dissector->offset[key->key_id] = key->offset;
> }
>
> /* Ensure that the dissector always includes control and basic key.
> * That way we are able to avoid handling lack of these in fast path.
> */
> - BUG_ON(!skb_flow_dissector_uses_key(flow_dissector,
> - FLOW_DISSECTOR_KEY_CONTROL));
> - BUG_ON(!skb_flow_dissector_uses_key(flow_dissector,
> - FLOW_DISSECTOR_KEY_BASIC));
> + BUG_ON(!dissector_uses_key(flow_dissector,
> + FLOW_DISSECTOR_KEY_CONTROL));
> + BUG_ON(!dissector_uses_key(flow_dissector,
> +
[PATCH] flow_dissector: Use 'const' where possible.
Signed-off-by: David S. Miller
---
include/linux/skbuff.h| 8 ++---
include/net/flow.h| 8 ++---
net/core/flow_dissector.c | 79 ---
3 files changed, 49 insertions(+), 46 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index eabfb81..2738d35 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1029,9 +1029,9 @@ static inline __u32 skb_get_hash(struct sk_buff *skb)
return skb->hash;
}
-__u32 __skb_get_hash_flowi6(struct sk_buff *skb, struct flowi6 *fl6);
+__u32 __skb_get_hash_flowi6(struct sk_buff *skb, const struct flowi6 *fl6);
-static inline __u32 skb_get_hash_flowi6(struct sk_buff *skb, struct flowi6
*fl6)
+static inline __u32 skb_get_hash_flowi6(struct sk_buff *skb, const struct
flowi6 *fl6)
{
if (!skb->l4_hash && !skb->sw_hash) {
struct flow_keys keys;
@@ -1043,9 +1043,9 @@ static inline __u32 skb_get_hash_flowi6(struct sk_buff
*skb, struct flowi6 *fl6)
return skb->hash;
}
-__u32 __skb_get_hash_flowi4(struct sk_buff *skb, struct flowi4 *fl);
+__u32 __skb_get_hash_flowi4(struct sk_buff *skb, const struct flowi4 *fl);
-static inline __u32 skb_get_hash_flowi4(struct sk_buff *skb, struct flowi4
*fl4)
+static inline __u32 skb_get_hash_flowi4(struct sk_buff *skb, const struct
flowi4 *fl4)
{
if (!skb->l4_hash && !skb->sw_hash) {
struct flow_keys keys;
diff --git a/include/net/flow.h b/include/net/flow.h
index dafe97c..acd6a09 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -244,18 +244,18 @@ void flow_cache_flush(struct net *net);
void flow_cache_flush_deferred(struct net *net);
extern atomic_t flow_cache_genid;
-__u32 __get_hash_from_flowi6(struct flowi6 *fl6, struct flow_keys *keys);
+__u32 __get_hash_from_flowi6(const struct flowi6 *fl6, struct flow_keys *keys);
-static inline __u32 get_hash_from_flowi6(struct flowi6 *fl6)
+static inline __u32 get_hash_from_flowi6(const struct flowi6 *fl6)
{
struct flow_keys keys;
return __get_hash_from_flowi6(fl6, &keys);
}
-__u32 __get_hash_from_flowi4(struct flowi4 *fl4, struct flow_keys *keys);
+__u32 __get_hash_from_flowi4(const struct flowi4 *fl4, struct flow_keys *keys);
-static inline __u32 get_hash_from_flowi4(struct flowi4 *fl4)
+static inline __u32 get_hash_from_flowi4(const struct flowi4 *fl4)
{
struct flow_keys keys;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 345a040..d79699c 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -19,14 +19,14 @@
#include
#include
-static bool skb_flow_dissector_uses_key(struct flow_dissector *flow_dissector,
- enum flow_dissector_key_id key_id)
+static bool dissector_uses_key(const struct flow_dissector *flow_dissector,
+ enum flow_dissector_key_id key_id)
{
return flow_dissector->used_keys & (1 << key_id);
}
-static void skb_flow_dissector_set_key(struct flow_dissector *flow_dissector,
- enum flow_dissector_key_id key_id)
+static void dissector_set_key(struct flow_dissector *flow_dissector,
+ enum flow_dissector_key_id key_id)
{
flow_dissector->used_keys |= (1 << key_id);
}
@@ -51,20 +51,20 @@ void skb_flow_dissector_init(struct flow_dissector
*flow_dissector,
* boundaries of unsigned short.
*/
BUG_ON(key->offset > USHRT_MAX);
- BUG_ON(skb_flow_dissector_uses_key(flow_dissector,
- key->key_id));
+ BUG_ON(dissector_uses_key(flow_dissector,
+ key->key_id));
- skb_flow_dissector_set_key(flow_dissector, key->key_id);
+ dissector_set_key(flow_dissector, key->key_id);
flow_dissector->offset[key->key_id] = key->offset;
}
/* Ensure that the dissector always includes control and basic key.
* That way we are able to avoid handling lack of these in fast path.
*/
- BUG_ON(!skb_flow_dissector_uses_key(flow_dissector,
- FLOW_DISSECTOR_KEY_CONTROL));
- BUG_ON(!skb_flow_dissector_uses_key(flow_dissector,
- FLOW_DISSECTOR_KEY_BASIC));
+ BUG_ON(!dissector_uses_key(flow_dissector,
+ FLOW_DISSECTOR_KEY_CONTROL));
+ BUG_ON(!dissector_uses_key(flow_dissector,
+ FLOW_DISSECTOR_KEY_BASIC));
}
EXPORT_SYMBOL(skb_flow_dissector_init);
@@ -154,8 +154,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
FLOW_DISSECTOR_KEY_BASIC,
target_container);
- if (skb_flow_dissector_uses_key(flow_dissect
