* [PATCH net-next 0/3] s390: qeth patches for net-next
@ 2015-08-19 8:20 Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 1/3] qeth: remove extraneous length from %pM format Ursula Braun
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ursula Braun @ 2015-08-19 8:20 UTC (permalink / raw
To: davem; +Cc: schwidefsky, heiko.carstens, netdev, linux-s390, ursula.braun,
ubraun
From: Ursula Braun <ursula.braun@de.ibm.com>
Hi Dave,
here are some s390 related qeth patches for net-next
shortlog:
Eugene Crosser (1):
qeth: remove extraneous length from %pM format
Vaishali Thakkar (1):
qeth: Convert use of __constant_htons to htons
Lakhvich Dmitriy (1):
qeth: no write permission for readonly sysattr
Thanks,
Ursula
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next 1/3] qeth: remove extraneous length from %pM format
2015-08-19 8:20 [PATCH net-next 0/3] s390: qeth patches for net-next Ursula Braun
@ 2015-08-19 8:20 ` Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 3/3] qeth: no write permission for readonly sysattr Ursula Braun
2 siblings, 0 replies; 11+ messages in thread
From: Ursula Braun @ 2015-08-19 8:20 UTC (permalink / raw
To: davem
Cc: schwidefsky, heiko.carstens, netdev, linux-s390, ursula.braun,
ubraun, Eugene Crosser, Ursula Braun
From: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
Length specifier in the %pM format is not supported (at least, not
documented). Remove it, and also an extraneous '&' for the array.
Signed-off-by: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibmn.com>
Suggested-by: Joe Perches <joe@perches.com>
---
drivers/s390/net/qeth_l2_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 2e65b98..e0f0188 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1455,8 +1455,8 @@ static void qeth_bridge_emit_host_event(struct qeth_card *card,
env[i] = str[i]; i++;
}
if (code & IPA_ADDR_CHANGE_CODE_MACADDR) {
- snprintf(str[i], sizeof(str[i]), "MAC=%pM6",
- &addr_lnid->mac);
+ snprintf(str[i], sizeof(str[i]), "MAC=%pM",
+ addr_lnid->mac);
env[i] = str[i]; i++;
}
snprintf(str[i], sizeof(str[i]), "NTOK_BUSID=%x.%x.%04x",
--
2.3.8
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-19 8:20 [PATCH net-next 0/3] s390: qeth patches for net-next Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 1/3] qeth: remove extraneous length from %pM format Ursula Braun
@ 2015-08-19 8:20 ` Ursula Braun
2015-08-20 10:46 ` David Laight
2015-08-19 8:20 ` [PATCH net-next 3/3] qeth: no write permission for readonly sysattr Ursula Braun
2 siblings, 1 reply; 11+ messages in thread
From: Ursula Braun @ 2015-08-19 8:20 UTC (permalink / raw
To: davem
Cc: schwidefsky, heiko.carstens, netdev, linux-s390, ursula.braun,
ubraun, Vaishali Thakkar
From: Vaishali Thakkar <vthakkar1994@gmail.com>
In little endian cases, the macro htons unfolds to __swab16 which
provides special case for constants. In big endian cases,
__constant_htons and htons expand directly to the same expression.
So, replace __constant_htons with htons with the goal of getting
rid of the definition of __constant_htons completely.
The semantic patch that performs this transformation is as follows:
@@expression x;@@
- __constant_htons(x)
+ htons(x)
Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---
drivers/s390/net/qeth_l2_main.c | 2 +-
drivers/s390/net/qeth_l3_main.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index e0f0188..dce1538 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -272,7 +272,7 @@ static void qeth_l2_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
/* VSWITCH relies on the VLAN
* information to be present in
* the QDIO header */
- if (veth->h_vlan_proto == __constant_htons(ETH_P_8021Q)) {
+ if (veth->h_vlan_proto == htons(ETH_P_8021Q)) {
hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_VLAN;
hdr->hdr.l2.vlan_id = ntohs(veth->h_vlan_TCI);
}
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 70eb2f6..ecfe622 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1887,13 +1887,13 @@ static inline int qeth_l3_rebuild_skb(struct qeth_card *card,
case QETH_CAST_MULTICAST:
switch (prot) {
#ifdef CONFIG_QETH_IPV6
- case __constant_htons(ETH_P_IPV6):
+ case htons(ETH_P_IPV6):
ndisc_mc_map((struct in6_addr *)
skb->data + 24,
tg_addr, card->dev, 0);
break;
#endif
- case __constant_htons(ETH_P_IP):
+ case htons(ETH_P_IP):
ip_hdr = (struct iphdr *)skb->data;
ip_eth_mc_map(ip_hdr->daddr, tg_addr);
break;
--
2.3.8
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next 3/3] qeth: no write permission for readonly sysattr
2015-08-19 8:20 [PATCH net-next 0/3] s390: qeth patches for net-next Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 1/3] qeth: remove extraneous length from %pM format Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons Ursula Braun
@ 2015-08-19 8:20 ` Ursula Braun
2 siblings, 0 replies; 11+ messages in thread
From: Ursula Braun @ 2015-08-19 8:20 UTC (permalink / raw
To: davem
Cc: schwidefsky, heiko.carstens, netdev, linux-s390, ursula.braun,
ubraun, Lakhvich Dmitriy
From: Lakhvich Dmitriy <ldmitriy@ru.ibm.com>
User is not allowed to write into bridge_state sysfs file.
Fixed attribute not mislead the user
Signed-off-by: Lakhvich Dmitriy <ldmitriy@ru.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Reported-by: Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
Reviewed-by: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
Reviewed-by: Thomas Richter <tmricht@de.ibm.com>
---
drivers/s390/net/qeth_l2_sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/s390/net/qeth_l2_sys.c b/drivers/s390/net/qeth_l2_sys.c
index 52673cd..692db49 100644
--- a/drivers/s390/net/qeth_l2_sys.c
+++ b/drivers/s390/net/qeth_l2_sys.c
@@ -109,7 +109,7 @@ static ssize_t qeth_bridge_port_state_show(struct device *dev,
return qeth_bridge_port_role_state_show(dev, attr, buf, 1);
}
-static DEVICE_ATTR(bridge_state, 0644, qeth_bridge_port_state_show,
+static DEVICE_ATTR(bridge_state, 0444, qeth_bridge_port_state_show,
NULL);
static ssize_t qeth_bridgeport_hostnotification_show(struct device *dev,
--
2.3.8
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-19 8:20 ` [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons Ursula Braun
@ 2015-08-20 10:46 ` David Laight
2015-08-20 11:43 ` Ursula Braun
0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2015-08-20 10:46 UTC (permalink / raw
To: 'Ursula Braun', davem@davemloft.net
Cc: schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com,
netdev@vger.kernel.org, linux-s390@vger.kernel.org,
ursula.braun@de.ibm.com, Vaishali Thakkar
From: Ursula Braun
> Sent: 19 August 2015 09:21
> In little endian cases, the macro htons unfolds to __swab16 which
> provides special case for constants. In big endian cases,
> __constant_htons and htons expand directly to the same expression.
> So, replace __constant_htons with htons with the goal of getting
> rid of the definition of __constant_htons completely.
...
> diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
> index 70eb2f6..ecfe622 100644
> --- a/drivers/s390/net/qeth_l3_main.c
> +++ b/drivers/s390/net/qeth_l3_main.c
> @@ -1887,13 +1887,13 @@ static inline int qeth_l3_rebuild_skb(struct qeth_card *card,
> case QETH_CAST_MULTICAST:
> switch (prot) {
> #ifdef CONFIG_QETH_IPV6
> - case __constant_htons(ETH_P_IPV6):
> + case htons(ETH_P_IPV6):
I didn't think htons() was 'constant enough' to be used as a case label.
Using byteswapped constants in a case statement can change it from being
implemented as a jump table to a branch tree.
This might be more expensive than byteswapping the value (even on systems
that don't have cheap byteswap instructions).
David
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-20 10:46 ` David Laight
@ 2015-08-20 11:43 ` Ursula Braun
2015-08-20 11:51 ` David Laight
0 siblings, 1 reply; 11+ messages in thread
From: Ursula Braun @ 2015-08-20 11:43 UTC (permalink / raw
To: David Laight
Cc: davem@davemloft.net, schwidefsky@de.ibm.com,
heiko.carstens@de.ibm.com, netdev@vger.kernel.org,
linux-s390@vger.kernel.org, ursula.braun@de.ibm.com,
Vaishali Thakkar
On Thu, 2015-08-20 at 10:46 +0000, David Laight wrote:
> From: Ursula Braun
> > Sent: 19 August 2015 09:21
> > In little endian cases, the macro htons unfolds to __swab16 which
> > provides special case for constants. In big endian cases,
> > __constant_htons and htons expand directly to the same expression.
> > So, replace __constant_htons with htons with the goal of getting
> > rid of the definition of __constant_htons completely.
> ...
> > diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
> > index 70eb2f6..ecfe622 100644
> > --- a/drivers/s390/net/qeth_l3_main.c
> > +++ b/drivers/s390/net/qeth_l3_main.c
> > @@ -1887,13 +1887,13 @@ static inline int qeth_l3_rebuild_skb(struct qeth_card *card,
> > case QETH_CAST_MULTICAST:
> > switch (prot) {
> > #ifdef CONFIG_QETH_IPV6
> > - case __constant_htons(ETH_P_IPV6):
> > + case htons(ETH_P_IPV6):
>
> I didn't think htons() was 'constant enough' to be used as a case label.
>
> Using byteswapped constants in a case statement can change it from being
> implemented as a jump table to a branch tree.
> This might be more expensive than byteswapping the value (even on systems
> that don't have cheap byteswap instructions).
>
> David
>
For big endian systems both __constant_htons(x) and htons(x) are
resolved to ((__force __be16)(__u16)(x)). Thus I do not see a reason to
reject the patch proposal from Vaishali Thakkar.
Ursula
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-20 11:43 ` Ursula Braun
@ 2015-08-20 11:51 ` David Laight
2015-08-20 13:53 ` Ursula Braun
0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2015-08-20 11:51 UTC (permalink / raw
To: 'Ursula Braun'
Cc: davem@davemloft.net, schwidefsky@de.ibm.com,
heiko.carstens@de.ibm.com, netdev@vger.kernel.org,
linux-s390@vger.kernel.org, ursula.braun@de.ibm.com,
Vaishali Thakkar
From: Ursula Braun [mailto:ubraun@linux.vnet.ibm.com]
> Sent: 20 August 2015 12:44
> On Thu, 2015-08-20 at 10:46 +0000, David Laight wrote:
> > From: Ursula Braun
> > > Sent: 19 August 2015 09:21
> > > In little endian cases, the macro htons unfolds to __swab16 which
> > > provides special case for constants. In big endian cases,
> > > __constant_htons and htons expand directly to the same expression.
> > > So, replace __constant_htons with htons with the goal of getting
> > > rid of the definition of __constant_htons completely.
> > ...
> > > diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
> > > index 70eb2f6..ecfe622 100644
> > > --- a/drivers/s390/net/qeth_l3_main.c
> > > +++ b/drivers/s390/net/qeth_l3_main.c
> > > @@ -1887,13 +1887,13 @@ static inline int qeth_l3_rebuild_skb(struct qeth_card *card,
> > > case QETH_CAST_MULTICAST:
> > > switch (prot) {
> > > #ifdef CONFIG_QETH_IPV6
> > > - case __constant_htons(ETH_P_IPV6):
> > > + case htons(ETH_P_IPV6):
> >
> > I didn't think htons() was 'constant enough' to be used as a case label.
> >
> > Using byteswapped constants in a case statement can change it from being
> > implemented as a jump table to a branch tree.
> > This might be more expensive than byteswapping the value (even on systems
> > that don't have cheap byteswap instructions).
> >
> > David
> >
> For big endian systems both __constant_htons(x) and htons(x) are
> resolved to ((__force __be16)(__u16)(x)). Thus I do not see a reason to
> reject the patch proposal from Vaishali Thakkar.
Look at a little-endian one (eg amd64).
I think you'll find a C ?: expression that uses __builtin_constant() to
select between an expression the compiler can evaluate and a call to an
inline function that uses some appropriate asm.
The latter isn't a compile-time constant so can't be used as a case
label or as an initialiser.
David
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-20 11:51 ` David Laight
@ 2015-08-20 13:53 ` Ursula Braun
2015-08-20 17:58 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Ursula Braun @ 2015-08-20 13:53 UTC (permalink / raw
To: David Laight
Cc: davem@davemloft.net, schwidefsky@de.ibm.com,
heiko.carstens@de.ibm.com, netdev@vger.kernel.org,
linux-s390@vger.kernel.org, ursula.braun@de.ibm.com,
Vaishali Thakkar
On Thu, 2015-08-20 at 11:51 +0000, David Laight wrote:
> From: Ursula Braun [mailto:ubraun@linux.vnet.ibm.com]
> > Sent: 20 August 2015 12:44
> > On Thu, 2015-08-20 at 10:46 +0000, David Laight wrote:
> > > From: Ursula Braun
> > > > Sent: 19 August 2015 09:21
> > > > In little endian cases, the macro htons unfolds to __swab16 which
> > > > provides special case for constants. In big endian cases,
> > > > __constant_htons and htons expand directly to the same expression.
> > > > So, replace __constant_htons with htons with the goal of getting
> > > > rid of the definition of __constant_htons completely.
> > > ...
> > > > diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
> > > > index 70eb2f6..ecfe622 100644
> > > > --- a/drivers/s390/net/qeth_l3_main.c
> > > > +++ b/drivers/s390/net/qeth_l3_main.c
> > > > @@ -1887,13 +1887,13 @@ static inline int qeth_l3_rebuild_skb(struct qeth_card *card,
> > > > case QETH_CAST_MULTICAST:
> > > > switch (prot) {
> > > > #ifdef CONFIG_QETH_IPV6
> > > > - case __constant_htons(ETH_P_IPV6):
> > > > + case htons(ETH_P_IPV6):
> > >
> > > I didn't think htons() was 'constant enough' to be used as a case label.
> > >
> > > Using byteswapped constants in a case statement can change it from being
> > > implemented as a jump table to a branch tree.
> > > This might be more expensive than byteswapping the value (even on systems
> > > that don't have cheap byteswap instructions).
> > >
> > > David
> > >
> > For big endian systems both __constant_htons(x) and htons(x) are
> > resolved to ((__force __be16)(__u16)(x)). Thus I do not see a reason to
> > reject the patch proposal from Vaishali Thakkar.
>
> Look at a little-endian one (eg amd64).
> I think you'll find a C ?: expression that uses __builtin_constant() to
> select between an expression the compiler can evaluate and a call to an
> inline function that uses some appropriate asm.
> The latter isn't a compile-time constant so can't be used as a case
> label or as an initialiser.
>
> David
>
>
qeth is an s390-driver, and s390 is a big-endian architecture. Thus
arguments valid for little-endian do not apply to qeth-code.
Ursula
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-20 13:53 ` Ursula Braun
@ 2015-08-20 17:58 ` David Miller
2015-08-20 18:58 ` Vaishali Thakkar
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2015-08-20 17:58 UTC (permalink / raw
To: ubraun
Cc: David.Laight, schwidefsky, heiko.carstens, netdev, linux-s390,
ursula.braun, vthakkar1994
From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Thu, 20 Aug 2015 15:53:42 +0200
> qeth is an s390-driver, and s390 is a big-endian architecture. Thus
> arguments valid for little-endian do not apply to qeth-code.
You can not throw out generally good tree-wide conventions just because
it happens to work on your platform.
In fact, I would be really thrilled if some of this code could be compiled
on other platforms via COMPILE_TEST or similar, especially the socket code
and drivers.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-20 17:58 ` David Miller
@ 2015-08-20 18:58 ` Vaishali Thakkar
2015-08-20 20:56 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Vaishali Thakkar @ 2015-08-20 18:58 UTC (permalink / raw
To: David Miller
Cc: Ursula Braun, David.Laight, Martin Schwidefsky, Heiko Carstens,
netdev, linux-s390, Ursula Braun
On Thu, Aug 20, 2015 at 11:28 PM, David Miller <davem@davemloft.net> wrote:
> From: Ursula Braun <ubraun@linux.vnet.ibm.com>
> Date: Thu, 20 Aug 2015 15:53:42 +0200
>
>> qeth is an s390-driver, and s390 is a big-endian architecture. Thus
>> arguments valid for little-endian do not apply to qeth-code.
>
> You can not throw out generally good tree-wide conventions just because
> it happens to work on your platform.
>
> In fact, I would be really thrilled if some of this code could be compiled
> on other platforms via COMPILE_TEST or similar, especially the socket code
> and drivers.
I have test-compiled this patch on little-endian architecture. I used
this script for cross compilation:
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
And it worked fine for me.
--
Vaishali
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons
2015-08-20 18:58 ` Vaishali Thakkar
@ 2015-08-20 20:56 ` David Miller
0 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2015-08-20 20:56 UTC (permalink / raw
To: vthakkar1994
Cc: ubraun, David.Laight, schwidefsky, heiko.carstens, netdev,
linux-s390, ursula.braun
From: Vaishali Thakkar <vthakkar1994@gmail.com>
Date: Fri, 21 Aug 2015 00:28:59 +0530
> On Thu, Aug 20, 2015 at 11:28 PM, David Miller <davem@davemloft.net> wrote:
>> From: Ursula Braun <ubraun@linux.vnet.ibm.com>
>> Date: Thu, 20 Aug 2015 15:53:42 +0200
>>
>>> qeth is an s390-driver, and s390 is a big-endian architecture. Thus
>>> arguments valid for little-endian do not apply to qeth-code.
>>
>> You can not throw out generally good tree-wide conventions just because
>> it happens to work on your platform.
>>
>> In fact, I would be really thrilled if some of this code could be compiled
>> on other platforms via COMPILE_TEST or similar, especially the socket code
>> and drivers.
>
> I have test-compiled this patch on little-endian architecture. I used
> this script for cross compilation:
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
If you cross compiled "from" a little endian architecture, that doesn't
mean anything.
What matters is if you compiled this code "targetting" a little-endian
architecture which currently is not possible.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-08-20 20:56 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-19 8:20 [PATCH net-next 0/3] s390: qeth patches for net-next Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 1/3] qeth: remove extraneous length from %pM format Ursula Braun
2015-08-19 8:20 ` [PATCH net-next 2/3] qeth: Convert use of __constant_htons to htons Ursula Braun
2015-08-20 10:46 ` David Laight
2015-08-20 11:43 ` Ursula Braun
2015-08-20 11:51 ` David Laight
2015-08-20 13:53 ` Ursula Braun
2015-08-20 17:58 ` David Miller
2015-08-20 18:58 ` Vaishali Thakkar
2015-08-20 20:56 ` David Miller
2015-08-19 8:20 ` [PATCH net-next 3/3] qeth: no write permission for readonly sysattr Ursula Braun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.