[PATCH] block: aoe: replace use of __constant_htons to htons

[PATCH] block: aoe: replace use of __constant_htons to htons

[Amey Narkhede]

The macro htons expands to __swab16 which has special
case for constants in little endian case. In big
endian case both __constant_htons and htons macros
expand to the same code. So, replace __constant_htons
with htons to get rid of the definition of __constant_htons
completely.

Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
---
 drivers/block/aoe/aoecmd.c | 2 +-
 drivers/block/aoe/aoenet.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 313f0b946fe2..7a5374a57b55 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -69,7 +69,7 @@ new_skb(ulong len)
 		skb_reserve(skb, MAX_HEADER);
 		skb_reset_mac_header(skb);
 		skb_reset_network_header(skb);
-		skb->protocol = __constant_htons(ETH_P_AOE);
+		skb->protocol = htons(ETH_P_AOE);
 		skb_checksum_none_assert(skb);
 	}
 	return skb;
diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
index 63773a90581d..2532e35774e0 100644
--- a/drivers/block/aoe/aoenet.c
+++ b/drivers/block/aoe/aoenet.c
@@ -192,7 +192,7 @@ aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt,
 }

 static struct packet_type aoe_pt __read_mostly = {
-	.type = __constant_htons(ETH_P_AOE),
+	.type = htons(ETH_P_AOE),
 	.func = aoenet_rcv,
 };

@@ -220,4 +220,3 @@ aoenet_exit(void)
 	skb_queue_purge(&skbtxq);
 	dev_remove_pack(&aoe_pt);
 }
-
--
2.29.2

RE: [PATCH] block: aoe: replace use of __constant_htons to htons

[David Laight]

From: Amey Narkhede
> Sent: 20 December 2020 16:46
> 
> The macro htons expands to __swab16 which has special
> case for constants in little endian case. In big
> endian case both __constant_htons and htons macros
> expand to the same code. So, replace __constant_htons
> with htons to get rid of the definition of __constant_htons
> completely.
> 
...
>  static struct packet_type aoe_pt __read_mostly = {
> -	.type = __constant_htons(ETH_P_AOE),
> +	.type = htons(ETH_P_AOE),
>  	.func = aoenet_rcv,

Does this cause grief if someone is doing a COMPILE_TEST on LE?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] block: aoe: replace use of __constant_htons to htons

['Amey Narkhede']

[-- Attachment #1: Type: text/plain, Size: 2682 bytes --]

On 20/12/21 09:23AM, David Laight wrote:
> From: 'Amey Narkhede' <ameynarkhede03@gmail.com>
> > Sent: 20 December 2020 20:20
> > To: David Laight <David.Laight@ACULAB.COM>
> > Subject: Re: [PATCH] block: aoe: replace use of __constant_htons to htons
> >
> > On 20/12/20 07:35PM, David Laight wrote:
> > > From: Amey Narkhede
> > > > Sent: 20 December 2020 16:46
> > > >
> > > > The macro htons expands to __swab16 which has special
> > > > case for constants in little endian case. In big
> > > > endian case both __constant_htons and htons macros
> > > > expand to the same code. So, replace __constant_htons
> > > > with htons to get rid of the definition of __constant_htons
> > > > completely.
> > > >
> > > ...
> > > >  static struct packet_type aoe_pt __read_mostly = {
> > > > -	.type = __constant_htons(ETH_P_AOE),
> > > > +	.type = htons(ETH_P_AOE),
> > > >  	.func = aoenet_rcv,
> > >
> > > Does this cause grief if someone is doing a COMPILE_TEST on LE?
> > >
> > > 	David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> > >
> > I did COMPILE_TEST on my x86_64 machine and it compiled
> > without any problems.
> > I assume that was the point or am I missing
> > something? I'm a beginner contributor so please
> > let me know if there any mistakes.
>
> It depends on exactly how the bswap functions are defined.
>
> You can't have anything that contains a function call (or asm) for a
> static initialiser or case label.
> The bswap functions have used the builtin_constant() check to switch
> between an asm block and C expression.
> So the C expression is used for constants - so evaluated by the compiler,
> and the asm for variables.
> Last I looked this wasn't valid for initialisers - so the __constant_bswap()
> exists for these cases.
>
> Now, it might be that, on more recent gcc __builtin_bswap() is used.
> This might be valid for constants in initialisers.
> Whether the same is true for CLANG is another issue.
>
> Basically it is all complex and partially historic.
>
> So you may need to use __constant_htons() for initialisers
> and case labels.
> For constants in code it doesn't matter whether you use the __constant form.
> For variables in code the __constant form is likely to generate horrid code.
>
> FWIW you'll get more credit for finding real bugs than stylistic changes.
>
> 	David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
Thanks for thee detail explanation.
Actually I was submitting stylistic changes to get
familiar with email and patch based workflows

Amey

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] block: aoe: replace use of __constant_htons to htons

['Amey Narkhede']

[-- Attachment #0: Type: message/rfc822, Size: 1547 bytes --]

From: 'Amey Narkhede' <ameynarkhede03@gmail.com>
To: David Laight <David.Laight@aculab.com>
Subject: Re: [PATCH] block: aoe: replace use of __constant_htons to htons
Date: Mon, 21 Dec 2020 01:50:23 +0530
Message-ID: <20201220202019.j7x64yahapgilr7u@archlinux>

On 20/12/20 07:35PM, David Laight wrote:
> From: Amey Narkhede
> > Sent: 20 December 2020 16:46
> >
> > The macro htons expands to __swab16 which has special
> > case for constants in little endian case. In big
> > endian case both __constant_htons and htons macros
> > expand to the same code. So, replace __constant_htons
> > with htons to get rid of the definition of __constant_htons
> > completely.
> >
> ...
> >  static struct packet_type aoe_pt __read_mostly = {
> > -	.type = __constant_htons(ETH_P_AOE),
> > +	.type = htons(ETH_P_AOE),
> >  	.func = aoenet_rcv,
>
> Does this cause grief if someone is doing a COMPILE_TEST on LE?
>
> 	David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
I did COMPILE_TEST on my x86_64 machine and it compiled
without any problems.
I assume that was the point or am I missing
something? I'm a beginner contributor so please
let me know if there any mistakes.

Amey