[PATCH] Bluetooth: Delete error messages for failed memory allocations in two functions

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 22 May 2017 08:42:28 +0200

Omit two extra messages for memory allocation failures in these functions.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 net/bluetooth/ecdh_helper.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/ecdh_helper.c b/net/bluetooth/ecdh_helper.c
index 24d4e60f8c48..c7b1a9aee579 100644
--- a/net/bluetooth/ecdh_helper.c
+++ b/net/bluetooth/ecdh_helper.c
@@ -92,8 +92,6 @@ bool compute_ecdh_secret(const u8 public_key[64], const u8 
private_key[32],
-   if (!buf) {
-   pr_err("alg: kpp: Failed to allocate %d bytes for buf\n",
-  buf_len);
+   if (!buf)
goto free_req;
-   }
+
crypto_ecdh_encode_key(buf, buf_len, &p);
 
/* Set A private Key */
@@ -173,8 +171,5 @@ bool generate_ecdh_keys(u8 public_key[64], u8 
private_key[32])
-   if (!buf) {
-   pr_err("alg: kpp: Failed to allocate %d bytes for buf\n",
-  buf_len);
+   if (!buf)
goto free_req;
-   }
 
do {
if (tries++ >= max_tries)
-- 
2.13.0




Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers

2017-05-21 Thread Jesper Dangaard Brouer
On Sun, 21 May 2017 15:10:29 -0700
Tom Herbert  wrote:

> On Sun, May 21, 2017 at 9:04 AM, Jesper Dangaard Brouer
>  wrote:
> > On Sat, 20 May 2017 09:16:09 -0700
> > Tom Herbert  wrote:
> >  
> >> > +/* XDP rxhash have an associated type, which is related to the RSS
> >> > + * (Receive Side Scaling) standard, but NIC HW have different mapping
> >> > + * and support. Thus, create mapping that is interesting for XDP.  XDP
> >> > + * would primarly want insight into L3 and L4 protocol info.
> >> > + *
> >> > + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
> >> > + *
> >> > + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
> >> > + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
> >> > + */
> >> > +#define XDP_HASH(x)((x) & ((1ULL << 32)-1))
> >> > +#define XDP_HASH_TYPE(x)   ((x) >> 32)
> >> > +
> >> > +#define XDP_HASH_TYPE_L3_SHIFT 0
> >> > +#define XDP_HASH_TYPE_L3_BITS  3
> >> > +#define XDP_HASH_TYPE_L3_MASK  ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
> >> > +#define XDP_HASH_TYPE_L3(x)((x) & XDP_HASH_TYPE_L3_MASK)
> >> > +enum {
> >> > +   XDP_HASH_TYPE_L3_IPV4 = 1,
> >> > +   XDP_HASH_TYPE_L3_IPV6,
> >> > +};
> >> > +
> >> > +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
> >> > +#define XDP_HASH_TYPE_L4_BITS  5
> >> > +#define XDP_HASH_TYPE_L4_MASK  \
> >> > +   (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
> >> > +#define XDP_HASH_TYPE_L4(x)((x) & XDP_HASH_TYPE_L4_MASK)
> >> > +enum {
> >> > +   _XDP_HASH_TYPE_L4_TCP = 1,
> >> > +   _XDP_HASH_TYPE_L4_UDP,
> >> > +};
> >> > +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << 
> >> > XDP_HASH_TYPE_L4_SHIFT)
> >> > +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << 
> >> > XDP_HASH_TYPE_L4_SHIFT)
> >> > +  
> >> Hi Jesper,
> >>
> >> Why do we need these indicators for protocol specific hash? It seems
> >> like L4 and L3 is useful differentiation and protocol agnostic (I'm
> >> still holding out hope that SCTP will be deployed some day ;-) )  
> >
> > I'm not sure I understood the question fully, but let me try to answer
> > anyway.  To me it seems obvious that you would want to know the
> > protocol/L4 type, as this helps avoid hash collisions between UDP and
> > TCP flows.  I can easily imagine someone constructing an UDP packet
> > that could hash collide with a given TCP flow.
> >
> > And yes, i40 support matching SCTP, and we will create a
> > XDP_HASH_TYPE_L4_SCTP when adding XDP rxhash support for that driver.
> >  
> But where would this information be used? We don't save it in skbuff,
> don't use it in RPS, RFS. RSS doesn't use it for packet steering so
> the hash collision problem already exists at the device level. If
> there is a collision problem between two protocols then maybe hash
> should be over 5-tuple instead...

One use-case (I heard at a customer) was that they had (web-)servers
that didn't serve any UDP traffic, thus they simply block/drop all
incoming UDP on the service NIC (as an ACL in the switch). (The servers
own DNS lookups and NTP goes through the management NIC to internal
DNS/NTP servers).

Another use-case: Inside an XDP/bpf program is can be used for
splitting protocol processing, into different tail calls, before even
touching packet-data.  I can imagine the bpf TCP handling code is
larger, thus an optimization is to have a separate tail call for the
UDP protocol handling.  One could also transfer/queue all TCP traffic
to other CPU(s) like RPS, just without touching packet memory.


This info is saved in the skb, but due to space constrains, it is
reduced to a single bit, namely skb->l4_hash, iif some
RSS-proto/XDP_HASH_TYPE_L4_* bit was set.  And the network stack do use
and react on this.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


[PATCH 4750/4750] net: ipv4: tcp: fixed comment coding style issue

2017-05-21 Thread Rohit Chavan
Fixed a coding style issue

Signed-off-by: Rohit Chavan 
---
 net/ipv4/tcp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 1e4c76d..87b0296 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2183,7 +2183,7 @@ void tcp_close(struct sock *sk, long timeout)
 
 
/* Now socket is owned by kernel and we acquire BH lock
-  to finish close. No need to check for user refs.
+*  to finish close. No need to check for user refs.
 */
local_bh_disable();
bh_lock_sock(sk);
@@ -2471,7 +2471,8 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
case TCP_MAXSEG:
/* Values greater than interface MTU won't take effect. However
 * at the point when this call is done we typically don't yet
-* know which interface is going to be used */
+* know which interface is going to be used
+*/
if (val && (val < TCP_MIN_MSS || val > MAX_TCP_WINDOW)) {
err = -EINVAL;
break;
-- 
2.7.4



[PATCH 2/2] ieee802154: ca8210: Delete an error message for a failed memory allocation in ca8210_skb_rx()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 22 May 2017 08:03:17 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/net/ieee802154/ca8210.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 25ed11bb5ed3..f6df75e80a60 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -1808,10 +1808,9 @@ static int ca8210_skb_rx(
 
/* Allocate mtu size buffer for every rx packet */
skb = dev_alloc_skb(IEEE802154_MTU + sizeof(hdr));
-   if (!skb) {
-   dev_crit(&priv->spi->dev, "dev_alloc_skb failed\n");
+   if (!skb)
return -ENOMEM;
-   }
+
skb_reserve(skb, sizeof(hdr));
 
msdulen = data_ind[22]; /* msdu_length */
-- 
2.13.0



[PATCH 1/2] ieee802154: ca8210: Delete an error message for a failed memory allocation in ca8210_probe()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 22 May 2017 07:32:46 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/net/ieee802154/ca8210.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 25fd3b04b3c0..25ed11bb5ed3 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -3143,10 +3143,6 @@ static int ca8210_probe(struct spi_device *spi_device)
 
pdata = kmalloc(sizeof(*pdata), GFP_KERNEL);
if (!pdata) {
-   dev_crit(
-   &spi_device->dev,
-   "Could not allocate platform data\n"
-   );
ret = -ENOMEM;
goto error;
}
-- 
2.13.0



[PATCH 0/2] ieee802154: ca8210: Adjustments for two function implementations

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 22 May 2017 08:08:04 +0200

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete an error message for a failed memory allocation in ca8210_probe()
  Delete an error message for a failed memory allocation in ca8210_skb_rx()

 drivers/net/ieee802154/ca8210.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

-- 
2.13.0



[PATCH 2/2] usbnet: Improve a size determination in usbnet_write_cmd_async()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 22 May 2017 06:42:33 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/net/usb/usbnet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 802ba68d37d1..b73c2a40501c 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -2129,6 +2129,6 @@ int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 
reqtype,
}
 
-   req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
+   req = kmalloc(sizeof(*req), GFP_ATOMIC);
if (!req)
goto fail_free_buf;
 
-- 
2.13.0



[PATCH 1/2] usbnet: Delete an error message for a failed memory allocation in usbnet_write_cmd_async()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 22 May 2017 06:33:48 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/net/usb/usbnet.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 79048e72c1bd..802ba68d37d1 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -2124,10 +2124,7 @@ int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, 
u8 reqtype,
 
if (data) {
buf = kmemdup(data, size, GFP_ATOMIC);
-   if (!buf) {
-   netdev_err(dev->net, "Error allocating buffer"
-  " in %s!\n", __func__);
+   if (!buf)
goto fail_free;
-   }
}
 
-- 
2.13.0



[PATCH 0/2] usbnet: Adjustments for usbnet_write_cmd_async()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 22 May 2017 07:04:03 +0200

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete an error message for a failed memory allocation
  Improve a size determination

 drivers/net/usb/usbnet.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.13.0



Fw: [Bug 195835] New: regression: tcp_fastretrans_alert

2017-05-21 Thread Stephen Hemminger


Begin forwarded message:

Date: Sun, 21 May 2017 08:53:17 +
From: bugzilla-dae...@bugzilla.kernel.org
To: step...@networkplumber.org
Subject: [Bug 195835] New: regression: tcp_fastretrans_alert


https://bugzilla.kernel.org/show_bug.cgi?id=195835

Bug ID: 195835
   Summary: regression: tcp_fastretrans_alert
   Product: Networking
   Version: 2.5
Kernel Version: 4.11.2
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: high
  Priority: P1
 Component: IPV4
  Assignee: step...@networkplumber.org
  Reporter: nanericw...@gmail.com
Regression: No

The issue can be reproduced in 4.11 running netfilter and Transmission
BitTorrent. However 4.10.x has no such issue.

[May21 16:38] [ cut here ]
[  +0.44] WARNING: CPU: 3 PID: 0 at net/ipv4/tcp_input.c:2819
tcp_fastretrans_alert+0x7ff/0xa10
[  +0.06] Modules linked in: xt_TPROXY iptable_mangle xt_owner xt_REDIRECT
nf_nat_redirect xt_set af_packet iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4
nf_nat_ipv4 nf_nat nf_conntrack iptable_filter ip_set_hash_net i
[  +0.000100] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.11.2 #2
[  +0.04] Hardware name: BIOSTAR Group A68N-5000/A68N-5000, BIOS 4.6.5
04/22/2014
[  +0.02] Call Trace:
[  +0.07]  
[  +0.10]  ? dump_stack+0x46/0x5e
[  +0.09]  ? __warn+0xb4/0xd0
[  +0.06]  ? tcp_fastretrans_alert+0x7ff/0xa10
[  +0.06]  ? tcp_ack+0xe95/0x1448
[  +0.09]  ? __ip_flush_pending_frames.isra.51+0x70/0x70
[  +0.07]  ? tcp_rcv_established+0x1ae/0x6b0
[  +0.07]  ? tcp_v4_do_rcv+0x12b/0x1e8
[  +0.06]  ? tcp_v4_rcv+0x7c4/0x880
[  +0.11]  ? nf_nat_ipv4_fn+0x53/0x150 [nf_nat_ipv4]
[  +0.07]  ? ip_local_deliver_finish+0x49/0x110
[  +0.06]  ? ip_local_deliver+0x66/0xd8
[  +0.06]  ? inet_del_offload+0x30/0x30
[  +0.08]  ? ip_sabotage_in+0x26/0x30 [br_netfilter]
[  +0.09]  ? nf_hook_slow+0x21/0xa0
[  +0.05]  ? ip_rcv+0x2e8/0x370
[  +0.06]  ? ip_local_deliver_finish+0x110/0x110
[  +0.08]  ? __netif_receive_skb_core+0x363/0x700
[  +0.07]  ? netif_receive_skb_internal+0x2a/0x98
[  +0.14]  ? br_pass_frame_up+0x60/0xd8 [bridge]
[  +0.12]  ? br_handle_frame_finish+0x132/0x330 [bridge]
[  +0.15]  ? nf_ct_get_tuple+0x56/0x90 [nf_conntrack]
[  +0.11]  ? br_pass_frame_up+0xd8/0xd8 [bridge]
[  +0.07]  ? br_nf_hook_thresh+0xa7/0xb8 [br_netfilter]
[  +0.09]  ? ipt_do_table+0x2b5/0x3e0 [ip_tables]
[  +0.07]  ? br_nf_pre_routing_finish+0x173/0x2f8 [br_netfilter]
[  +0.10]  ? br_pass_frame_up+0xd8/0xd8 [bridge]
[  +0.07]  ? nf_nat_ipv4_in+0x23/0x70 [nf_nat_ipv4]
[  +0.07]  ? br_nf_pre_routing+0x2c4/0x420 [br_netfilter]
[  +0.07]  ? br_nf_forward_arp+0x250/0x250 [br_netfilter]
[  +0.07]  ? nf_hook_slow+0x21/0xa0
[  +0.10]  ? br_handle_frame+0x1ae/0x2c8 [bridge]
[  +0.10]  ? br_pass_frame_up+0xd8/0xd8 [bridge]
[  +0.10]  ? br_handle_local_finish+0x38/0x38 [bridge]
[  +0.06]  ? __netif_receive_skb_core+0x1d7/0x700
[  +0.06]  ? netif_receive_skb_internal+0x2a/0x98
[  +0.06]  ? napi_gro_receive+0x6a/0xb0
[  +0.12]  ? rtl8169_poll+0x2c9/0x690 [r8169]
[  +0.07]  ? net_rx_action+0x1c6/0x2b8
[  +0.07]  ? __do_softirq+0xd8/0x200
[  +0.05]  ? irq_exit+0xa3/0xa8
[  +0.07]  ? do_IRQ+0x45/0xc0
[  +0.09]  ? common_interrupt+0x7f/0x7f
[  +0.03]  
[  +0.13]  ? acpi_idle_do_entry+0x2b/0xed0 [processor]
[  +0.07]  ? acpi_idle_enter+0xe1/0x288 [processor]
[  +0.09]  ? cpuidle_enter_state+0x128/0x1f0
[  +0.08]  ? do_idle+0x17a/0x1d8
[  +0.06]  ? cpu_startup_entry+0x68/0x70
[  +0.09]  ? start_secondary+0x138/0x158
[  +0.05]  ? start_cpu+0x14/0x14
[  +0.08] ---[ end trace ab2d840ff39c9793 ]---

-- 
You are receiving this mail because:
You are the assignee for the bug.


Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers

2017-05-21 Thread John Fastabend
On 05/21/2017 08:21 PM, Alexei Starovoitov wrote:
> On Sun, May 21, 2017 at 05:55:50PM +0200, Jesper Dangaard Brouer wrote:
>>> And it looks useful to me, but
>>
>>> 1. i'm worried that we'd be relying on something that mellanox didn't
>>>  implement in their drivers before. Was it tested and guarnteed to
>>>  exist in the future revisions of firmware? Is it cx4 or cx4-lx or cx5
>>>  feature?
>>
>> It is not a hidden mlx5 or specific feature.  Due to the Microsoft RSS
>> standard/requirements[2] most NICs actually implement this.
>>
>> [2] 
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-hashing-types
> 
> ...
> 
>>> 2. but the main concern that it is mellanox only feature. At least I cannot
>>> see anything like this in broadcom and intel nics
>>
>> All the drivers I looked at have support for an RSS hash type.
>> Including Broadcom[3] and Intel. Just grep after NETIF_F_RXHASH, and
>> follow data-structs.  The Intel i40 NIC have the most elaborate rss type
>> system (it can e.g. tell if this was SCTP).
>>
>> [3] 
>> http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h#L4198
> 
> yes and bnxt too.
> msft spec requires RSS to be configured in these different ways, but
> it doesn't mean that HW descriptor will have 'is_v4' and 'is_v6' bits set.
> imo this is mlx specific behavior.
> If you want to piggy back on msft spec and make linux rss to be configurable
> the same way, I guess that's fine, but imo it's orthogonal to xdp.
> 
>>> How about exposing 'struct mlx5_cqe64 *' to XDP programs as-is?
>>> We can make sure that XDP program does read only access into it and
>>> it will see cqe->rss_hash_result, cqe->rss_hash_type and everything else
>>> in there, but this will not be uapi and it will be pretty obvious
>>> to program authors that their programs are vendor specific.
>>
>> This sounds EXTREMELY dangerous to me... IHMO this will lead to vendor
>> lock-in.  As BPF program authors will become dependent on vendor
>> specific features, and their program are no longer portable to run on
>> other NICs.
>>
>> How are you going to avoid vendor lock-in with this model?
> 
> It looked to me that that was the intent of your patch set, hence
> counter proposal to make it much simpler.
> I'm not going to use vendor specific features. The proposal
> to expose hw rx descriptor as-is is for people who desperately want
> that info without burdening core xdp with it.
> 
>>> 'not uapi' here means that mellanox is free to change their HW descriptor
>>> and its contents as they wish.
>>
>> Hmmm... IMHO directly exposing the HW descriptor to userspace, will
>> limit vendors ability to change its contents.
> 
> kprobes can already look at hw rx descriptor.
> if somebody really wants to look into it, they have a way to do it already:
> - add kprobe to mlx5e_handle_rx_cqe(), look into cqe, store the outcome on a 
> side
> - use that info in the xdp program
> All I proposed is to make it first class citizen and avoid kprobe.
> 

Another solution is to have hardware prepend meta-data to the front of the
packet and have the XDP program read it out. Of course the hardware and
XDP program need to be in sync at this point, but it works today assuming
a mechanism to program hardware exists.

The nice part of the above is you push all the complexity of feature negotiation
and hardware initialization out of XDP core completely.

This would be my preferred solution, except I'm not sure if some hardware
would have issue with this.

.John



Re: [PATCH net-next] net: define PKTINFO timestamping option in all archs

2017-05-21 Thread Willem de Bruijn
On Sun, May 21, 2017 at 11:35 PM, David Miller  wrote:
> From: Willem de Bruijn 
> Date: Sun, 21 May 2017 23:31:47 -0400
>
>> From: Willem de Bruijn 
>>
>> The newly introduced timestamping option SCM_TIMESTAMPING_PKTINFO
>> is defined in asm-generic/socket.h. Also define it in the architecture
>> specific versions.
>>
>> Fixes: aad9c8c470f2 ("net: add new control message for incoming 
>> HW-timestamped packets")
>> Reported-by: Stephen Rothwell 
>> CC: Miroslav Lichvar 
>> Signed-off-by: Willem de Bruijn 
>
> I pushed a fix for this about 15 minutes ago :-).

Thanks, David!

Then ignore this one. Apologies for not spotting this sooner.


Re: linux-next: build failure after merge of the net-next tree

2017-05-21 Thread Willem de Bruijn
On Sun, May 21, 2017 at 9:16 PM, Stephen Rothwell  wrote:
> Hi all,
>
> After merging the net-next tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> net/socket.c: In function 'put_ts_pktinfo':
> net/socket.c:695:28: error: 'SCM_TIMESTAMPING_PKTINFO' undeclared (first use 
> in this function)
>   put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_PKTINFO,
> ^
>
> Caused by commit
>
>   aad9c8c470f2 ("net: add new control message for incoming HW-timestamped 
> packets")
>
> This probably broke every architecture that has its own
> arch//include/uapi/asm/socket.h that does not include
> include/uapi/asm-generic/socket.h :-(

Indeed. I added the architecture specific versions in patch

  http://patchwork.ozlabs.org/patch/765238/

That fixes the powerpc build for me. The new option is now
defined in every file that also defines the last added such
option SCM_TIMESTAMPING_OPT_STATS. Apologies for
missing this earlier.


Re: linux-next: build failure after merge of the net-next tree

2017-05-21 Thread Stephen Rothwell
Hi Dave,

On Sun, 21 May 2017 23:14:10 -0400 (EDT) David Miller  
wrote:
>
> From: Stephen Rothwell 
> Date: Mon, 22 May 2017 11:16:05 +1000
> 
> > After merging the net-next tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> > 
> > net/socket.c: In function 'put_ts_pktinfo':
> > net/socket.c:695:28: error: 'SCM_TIMESTAMPING_PKTINFO' undeclared (first 
> > use in this function)
> >   put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_PKTINFO,
> > ^
> > Caused by commit
> > 
> >   aad9c8c470f2 ("net: add new control message for incoming HW-timestamped 
> > packets")
> > 
> > This probably broke every architecture that has its own
> > arch//include/uapi/asm/socket.h that does not include
> > include/uapi/asm-generic/socket.h :-(
> > 
> > I have used the net-next tree from next-20170519 for today.  
> 
> I've just pushed the following, thanks for the report:

Looks good except:

> diff --git a/arch/parisc/include/uapi/asm/socket.h 
> b/arch/parisc/include/uapi/asm/socket.h
> index 5147018..784b871 100644
> --- a/arch/parisc/include/uapi/asm/socket.h
> +++ b/arch/parisc/include/uapi/asm/socket.h
> @@ -97,4 +97,6 @@
>  
>  #define SO_COOKIE0x4032
>  
> +#define SCM_TIMESTAMPING_PKTINFO 58

Does this need to be 0x4033 (or something)?

-- 
Cheers,
Stephen Rothwell


Re: [PATCH net-next] net: define PKTINFO timestamping option in all archs

2017-05-21 Thread David Miller
From: Willem de Bruijn 
Date: Sun, 21 May 2017 23:31:47 -0400

> From: Willem de Bruijn 
> 
> The newly introduced timestamping option SCM_TIMESTAMPING_PKTINFO
> is defined in asm-generic/socket.h. Also define it in the architecture
> specific versions.
> 
> Fixes: aad9c8c470f2 ("net: add new control message for incoming 
> HW-timestamped packets")
> Reported-by: Stephen Rothwell 
> CC: Miroslav Lichvar 
> Signed-off-by: Willem de Bruijn 

I pushed a fix for this about 15 minutes ago :-)


[PATCH net-next] net: define PKTINFO timestamping option in all archs

2017-05-21 Thread Willem de Bruijn
From: Willem de Bruijn 

The newly introduced timestamping option SCM_TIMESTAMPING_PKTINFO
is defined in asm-generic/socket.h. Also define it in the architecture
specific versions.

Fixes: aad9c8c470f2 ("net: add new control message for incoming HW-timestamped 
packets")
Reported-by: Stephen Rothwell 
CC: Miroslav Lichvar 
Signed-off-by: Willem de Bruijn 
---
 arch/alpha/include/uapi/asm/socket.h   | 2 ++
 arch/frv/include/uapi/asm/socket.h | 2 ++
 arch/ia64/include/uapi/asm/socket.h| 2 ++
 arch/m32r/include/uapi/asm/socket.h| 2 ++
 arch/mips/include/uapi/asm/socket.h| 2 ++
 arch/mn10300/include/uapi/asm/socket.h | 2 ++
 arch/parisc/include/uapi/asm/socket.h  | 2 ++
 arch/powerpc/include/uapi/asm/socket.h | 2 ++
 arch/s390/include/uapi/asm/socket.h| 2 ++
 arch/sparc/include/uapi/asm/socket.h   | 2 ++
 arch/xtensa/include/uapi/asm/socket.h  | 2 ++
 11 files changed, 22 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/socket.h 
b/arch/alpha/include/uapi/asm/socket.h
index 148d7a32754e..0926de63a62b 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -105,4 +105,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h 
b/arch/frv/include/uapi/asm/socket.h
index 1ccf45657472..e491ff08b9a9 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -98,5 +98,7 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h 
b/arch/ia64/include/uapi/asm/socket.h
index 2c3f4b48042a..86937241 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -107,4 +107,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h 
b/arch/m32r/include/uapi/asm/socket.h
index ae6548d29a18..5d97890a8704 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -98,4 +98,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h 
b/arch/mips/include/uapi/asm/socket.h
index 3418ec9c1c50..365ff51f033a 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -116,4 +116,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h 
b/arch/mn10300/include/uapi/asm/socket.h
index 4526e92301a6..d013c0da0256 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -98,4 +98,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h 
b/arch/parisc/include/uapi/asm/socket.h
index 514701840bd9..b893ca14fade 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -97,4 +97,6 @@
 
 #define SO_COOKIE  0x4032
 
+#define SCM_TIMESTAMPING_PKTINFO   0x4033
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/powerpc/include/uapi/asm/socket.h 
b/arch/powerpc/include/uapi/asm/socket.h
index 58e2ec0310fc..bc4ca72faf99 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -105,4 +105,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h 
b/arch/s390/include/uapi/asm/socket.h
index e8e5ecf673fd..fb9769d7e74e 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -104,4 +104,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h 
b/arch/sparc/include/uapi/asm/socket.h
index 3f4ad19d9ec7..5d673302fd41 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -94,6 +94,8 @@
 
 #define SO_COOKIE  0x003b
 
+#define SCM_TIMESTAMPING_PKTINFO   0x003c
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION 0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT   0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h 
b/arch/xtensa/include/uapi/asm/socket.h
index 1eb6d2fe70d3..982c2533f912 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -109,4 +109,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _XTENSA_SOCKET_H */
-- 
2.13.0.303.g4ebf302169-goog



Re: [Intel-wired-lan] [PATCH 0/4] Configuring traffic classes via new hardware offload mechanism in tc/mqprio

2017-05-21 Thread Or Gerlitz
On Mon, May 22, 2017 at 1:35 AM, Alexander Duyck
 wrote:
> On Sat, May 20, 2017 at 2:15 PM, Or Gerlitz  wrote:
>> On Sat, May 20, 2017 at 3:58 AM, Amritha Nambiar
>>  wrote:
>>> The following series introduces a new harware offload mode in tc/mqprio
>>
>> Wait, we have already a HW QoS model introduced by John F and Co
>> couple of years ago,  right?
>
> I assume you are referring to the ETS portion of DCBX? If so then yes
> we have something there, but there is a fairly high level of
> complexity and dependencies in order to enable that. What we have in
> mind doesn't really fit with DCBX and the problem is the spec calls
> out that you really have to have support for DCBX in order to make use
> of ETS. What we are trying to do here is provide a lightweight way of
> doing this configuration similar to how mqprio was already providing a
> lightweight way of enabling multiple traffic classes.

UAPI wise, we are talking on DCB, not DCBX, right? the X-portion comes
into play if some user-space entity run LLDP traffic and calls into
the kernel to configure (say) ETS. If there are some issues to use the
existing user-space tool (lldpad tool/daemon) to do DCB without X, one
can/should fix them or introduce another/simpler tool that in a
lightweight manner only configures things w.o exchanging.

So to clarify, the essence of this series is introducing a 2nd way to
configure ETS and rate limit?

>> Please elaborate in few sentence if you are extending it/replacing it,
>> why we want to do that and what are the implications on existing
>> applications UAPI wise.

> This is meant to be an extension of the existing structure. It can be
> ignored by the driver and instead only have the basic hw offload
> supported. In such a case the command will still return success but
> any rate limits and queue configuration can be ignored. In the case of
> the current implementation the queue configuration was already being
> ignored so we opted to re-purpose the "hw" flag so that if you pass a
> value greater than 1 and the driver doesn't recognize the value it has
> the option to just ignore the extra bits it doesn't recognize and
> return 1 as it did before for the hw flag in mqprio.

So the user asked to configure something and the kernel returned
success but the config was not plugged to the hw? sounds to me like
possible (probable) source of troubles and complaints.

>> Below you just say in the new mode Qos is configured with knobs XYZ --
>> this is way not enough

> Can you clarify what you are asking for here? Amritha included an
> example in patch 1 of a command line that enabled 2 traffic classes
> with rate limits. Is there something more specific you are looking for?

you were referring to the questions I posted, so we can continue the
discussion from here.


Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers

2017-05-21 Thread Alexei Starovoitov
On Sun, May 21, 2017 at 05:55:50PM +0200, Jesper Dangaard Brouer wrote:
> > And it looks useful to me, but
> 
> > 1. i'm worried that we'd be relying on something that mellanox didn't
> >  implement in their drivers before. Was it tested and guarnteed to
> >  exist in the future revisions of firmware? Is it cx4 or cx4-lx or cx5
> >  feature?
> 
> It is not a hidden mlx5 or specific feature.  Due to the Microsoft RSS
> standard/requirements[2] most NICs actually implement this.
> 
> [2] 
> https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-hashing-types

...

> > 2. but the main concern that it is mellanox only feature. At least I cannot
> > see anything like this in broadcom and intel nics
> 
> All the drivers I looked at have support for an RSS hash type.
> Including Broadcom[3] and Intel. Just grep after NETIF_F_RXHASH, and
> follow data-structs.  The Intel i40 NIC have the most elaborate rss type
> system (it can e.g. tell if this was SCTP).
> 
> [3] 
> http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h#L4198

yes and bnxt too.
msft spec requires RSS to be configured in these different ways, but
it doesn't mean that HW descriptor will have 'is_v4' and 'is_v6' bits set.
imo this is mlx specific behavior.
If you want to piggy back on msft spec and make linux rss to be configurable
the same way, I guess that's fine, but imo it's orthogonal to xdp.

> > How about exposing 'struct mlx5_cqe64 *' to XDP programs as-is?
> > We can make sure that XDP program does read only access into it and
> > it will see cqe->rss_hash_result, cqe->rss_hash_type and everything else
> > in there, but this will not be uapi and it will be pretty obvious
> > to program authors that their programs are vendor specific.
> 
> This sounds EXTREMELY dangerous to me... IHMO this will lead to vendor
> lock-in.  As BPF program authors will become dependent on vendor
> specific features, and their program are no longer portable to run on
> other NICs.
> 
> How are you going to avoid vendor lock-in with this model?

It looked to me that that was the intent of your patch set, hence
counter proposal to make it much simpler.
I'm not going to use vendor specific features. The proposal
to expose hw rx descriptor as-is is for people who desperately want
that info without burdening core xdp with it.

> > 'not uapi' here means that mellanox is free to change their HW descriptor
> > and its contents as they wish.
> 
> Hmmm... IMHO directly exposing the HW descriptor to userspace, will
> limit vendors ability to change its contents.

kprobes can already look at hw rx descriptor.
if somebody really wants to look into it, they have a way to do it already:
- add kprobe to mlx5e_handle_rx_cqe(), look into cqe, store the outcome on a 
side
- use that info in the xdp program
All I proposed is to make it first class citizen and avoid kprobe.



Re: linux-next: build failure after merge of the net-next tree

2017-05-21 Thread David Miller
From: Stephen Rothwell 
Date: Mon, 22 May 2017 11:16:05 +1000

> After merging the net-next tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> net/socket.c: In function 'put_ts_pktinfo':
> net/socket.c:695:28: error: 'SCM_TIMESTAMPING_PKTINFO' undeclared (first use 
> in this function)
>   put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_PKTINFO,
> ^
> Caused by commit
> 
>   aad9c8c470f2 ("net: add new control message for incoming HW-timestamped 
> packets")
> 
> This probably broke every architecture that has its own
> arch//include/uapi/asm/socket.h that does not include
> include/uapi/asm-generic/socket.h :-(
> 
> I have used the net-next tree from next-20170519 for today.

I've just pushed the following, thanks for the report:


net: Define SCM_TIMESTAMPING_PKTINFO on all architectures.

A definition was only provided for asm-generic/socket.h
using platforms, define it for the others as well

Reported-by: Stephen Rothwell 
Signed-off-by: David S. Miller 
---
 arch/alpha/include/uapi/asm/socket.h   | 2 ++
 arch/frv/include/uapi/asm/socket.h | 2 ++
 arch/ia64/include/uapi/asm/socket.h| 2 ++
 arch/m32r/include/uapi/asm/socket.h| 2 ++
 arch/mips/include/uapi/asm/socket.h| 2 ++
 arch/mn10300/include/uapi/asm/socket.h | 2 ++
 arch/parisc/include/uapi/asm/socket.h  | 2 ++
 arch/powerpc/include/uapi/asm/socket.h | 2 ++
 arch/s390/include/uapi/asm/socket.h| 2 ++
 arch/sparc/include/uapi/asm/socket.h   | 2 ++
 arch/xtensa/include/uapi/asm/socket.h  | 2 ++
 11 files changed, 22 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/socket.h 
b/arch/alpha/include/uapi/asm/socket.h
index 148d7a3..0926de6 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -105,4 +105,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h 
b/arch/frv/include/uapi/asm/socket.h
index 1ccf456..e491ff0 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -98,5 +98,7 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h 
b/arch/ia64/include/uapi/asm/socket.h
index 2c3f4b4..8693724 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -107,4 +107,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h 
b/arch/m32r/include/uapi/asm/socket.h
index ae6548d..5d97890 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -98,4 +98,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h 
b/arch/mips/include/uapi/asm/socket.h
index 3418ec9..365ff51 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -116,4 +116,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h 
b/arch/mn10300/include/uapi/asm/socket.h
index 4526e92..d013c0d 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -98,4 +98,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h 
b/arch/parisc/include/uapi/asm/socket.h
index 5147018..784b871 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -97,4 +97,6 @@
 
 #define SO_COOKIE  0x4032
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/powerpc/include/uapi/asm/socket.h 
b/arch/powerpc/include/uapi/asm/socket.h
index 58e2ec0..bc4ca72 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -105,4 +105,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h 
b/arch/s390/include/uapi/asm/socket.h
index e8e5ecf..fb9769d 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -104,4 +104,6 @@
 
 #define SO_COOKIE  57
 
+#define SCM_TIMESTAMPING_PKTINFO   58
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h 
b/arch/sparc/include/uapi/asm/socket.h
index 3f4ad19..5d67330 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -94,6 +94,8 @@
 
 #define SO_COOKIE  0x003b
 
+#define SCM_TIMESTAMPING_PKTINFO   0x003c
+
 /* Secur

[PATCH] H.245 ALG dropping packets

2017-05-21 Thread Blair Steven
We have a setup where two VoIP phones are communicating through a
router on a trusted LAN where the H.245 ALG is dropping some of the
traffic, so far as I can tell without good cause. These two devices
are configured differently, one with Fast Connect and one without -
this might be the reason, but from my (limited) understanding of ALGs
this isn't a good enough reason.

Does it ever make sense to drop packets in an ALG?

Blair Steven (1):
  Accept packets that the H.245 ALG can't process

 net/netfilter/nf_conntrack_h323_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.9.3



[PATCH] Accept packets that the H.245 ALG can't process

2017-05-21 Thread Blair Steven
When two VoIP end points are configured differently (fast connect /
not fast connect) the ALG was failing to find a matching expectation
and dropping packets in one direction.

Dropping packets not the job of an ALG, and as such the behaviour
has been changed to allow the packet to be send to the forwarding
engine.

Signed-off-by: Blair Steven 
---
 net/netfilter/nf_conntrack_h323_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_h323_main.c 
b/net/netfilter/nf_conntrack_h323_main.c
index 3bcdc71..6161375 100644
--- a/net/netfilter/nf_conntrack_h323_main.c
+++ b/net/netfilter/nf_conntrack_h323_main.c
@@ -625,7 +625,7 @@ static int h245_help(struct sk_buff *skb, unsigned int 
protoff,
   drop:
spin_unlock_bh(&nf_h323_lock);
nf_ct_helper_log(skb, ct, "cannot process H.245 message");
-   return NF_DROP;
+   return NF_ACCEPT;
 }
 
 //
@@ -1200,7 +1200,7 @@ static int q931_help(struct sk_buff *skb, unsigned int 
protoff,
   drop:
spin_unlock_bh(&nf_h323_lock);
nf_ct_helper_log(skb, ct, "cannot process Q.931 message");
-   return NF_DROP;
+   return NF_ACCEPT;
 }
 
 //
@@ -1785,7 +1785,7 @@ static int ras_help(struct sk_buff *skb, unsigned int 
protoff,
   drop:
spin_unlock_bh(&nf_h323_lock);
nf_ct_helper_log(skb, ct, "cannot process RAS message");
-   return NF_DROP;
+   return NF_ACCEPT;
 }
 
 //
-- 
2.9.3



linux-next: build failure after merge of the net-next tree

2017-05-21 Thread Stephen Rothwell
Hi all,

After merging the net-next tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

net/socket.c: In function 'put_ts_pktinfo':
net/socket.c:695:28: error: 'SCM_TIMESTAMPING_PKTINFO' undeclared (first use in 
this function)
  put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_PKTINFO,
^

Caused by commit

  aad9c8c470f2 ("net: add new control message for incoming HW-timestamped 
packets")

This probably broke every architecture that has its own
arch//include/uapi/asm/socket.h that does not include
include/uapi/asm-generic/socket.h :-(

I have used the net-next tree from next-20170519 for today.

-- 
Cheers,
Stephen Rothwell


Re: [Intel-wired-lan] [PATCH 0/4] Configuring traffic classes via new hardware offload mechanism in tc/mqprio

2017-05-21 Thread Alexander Duyck
On Sat, May 20, 2017 at 2:15 PM, Or Gerlitz  wrote:
> On Sat, May 20, 2017 at 3:58 AM, Amritha Nambiar
>  wrote:
>> The following series introduces a new harware offload mode in tc/mqprio
>
> Wait, we have already a HW QoS model introduced by John F and Co
> couple of years ago,  right?

I assume you are referring to the ETS portion of DCBX? If so then yes
we have something there, but there is a fairly high level of
complexity and dependencies in order to enable that. What we have in
mind doesn't really fit with DCBX and the problem is the spec calls
out that you really have to have support for DCBX in order to make use
of ETS. What we are trying to do here is provide a lightweight way of
doing this configuration similar to how mqprio was already providing a
lightweight way of enabling multiple traffic classes.

> Please elaborate in few sentence if you are extending it/replacing it,
> why we want to do that and what are the implications on existing
> applications UAPI wise.

This is meant to be an extension of the existing structure. It can be
ignored by the driver and instead only have the basic hw offload
supported. In such a case the command will still return success but
any rate limits and queue configuration can be ignored. In the case of
the current implementation the queue configuration was already being
ignored so we opted to re-purpose the "hw" flag so that if you pass a
value greater than 1 and the driver doesn't recognize the value it has
the option to just ignore the extra bits it doesn't recognize and
return 1 as it did before for the hw flag in mqprio.

> Below you just say in the new mode Qos is configured with knobs XYZ --
> this is way not enough

Can you clarify what you are asking for here? Amritha included an
example in patch 1 of a command line that enabled 2 traffic classes
with rate limits. Is there something more specific you are looking
for?

Thanks.

- Alex


Re: [PATCH 00/12] Netfilter/IPVS fixes for net

2017-05-21 Thread Pablo Neira Ayuso
On Sun, May 21, 2017 at 01:00:33PM -0400, David Miller wrote:
> From: Pablo Neira Ayuso 
> Date: Fri, 19 May 2017 10:33:41 +0200
> 
> > The following patchset contains Netfilter/IPVS fixes for your net tree,
> > they are:
>  ...
> > You can pull these changes from:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
> 
> Pulled, thanks Pablo.

Thanks David!

Could you merge net into net-next as well? I have several patches for
net-next that need to apply on these fixes. No rush BTW.


Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers

2017-05-21 Thread Tom Herbert
On Sun, May 21, 2017 at 9:04 AM, Jesper Dangaard Brouer
 wrote:
> On Sat, 20 May 2017 09:16:09 -0700
> Tom Herbert  wrote:
>
>> > +/* XDP rxhash have an associated type, which is related to the RSS
>> > + * (Receive Side Scaling) standard, but NIC HW have different mapping
>> > + * and support. Thus, create mapping that is interesting for XDP.  XDP
>> > + * would primarly want insight into L3 and L4 protocol info.
>> > + *
>> > + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
>> > + *
>> > + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
>> > + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
>> > + */
>> > +#define XDP_HASH(x)((x) & ((1ULL << 32)-1))
>> > +#define XDP_HASH_TYPE(x)   ((x) >> 32)
>> > +
>> > +#define XDP_HASH_TYPE_L3_SHIFT 0
>> > +#define XDP_HASH_TYPE_L3_BITS  3
>> > +#define XDP_HASH_TYPE_L3_MASK  ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
>> > +#define XDP_HASH_TYPE_L3(x)((x) & XDP_HASH_TYPE_L3_MASK)
>> > +enum {
>> > +   XDP_HASH_TYPE_L3_IPV4 = 1,
>> > +   XDP_HASH_TYPE_L3_IPV6,
>> > +};
>> > +
>> > +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
>> > +#define XDP_HASH_TYPE_L4_BITS  5
>> > +#define XDP_HASH_TYPE_L4_MASK  \
>> > +   (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
>> > +#define XDP_HASH_TYPE_L4(x)((x) & XDP_HASH_TYPE_L4_MASK)
>> > +enum {
>> > +   _XDP_HASH_TYPE_L4_TCP = 1,
>> > +   _XDP_HASH_TYPE_L4_UDP,
>> > +};
>> > +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << 
>> > XDP_HASH_TYPE_L4_SHIFT)
>> > +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << 
>> > XDP_HASH_TYPE_L4_SHIFT)
>> > +
>> Hi Jesper,
>>
>> Why do we need these indicators for protocol specific hash? It seems
>> like L4 and L3 is useful differentiation and protocol agnostic (I'm
>> still holding out hope that SCTP will be deployed some day ;-) )
>
> I'm not sure I understood the question fully, but let me try to answer
> anyway.  To me it seems obvious that you would want to know the
> protocol/L4 type, as this helps avoid hash collisions between UDP and
> TCP flows.  I can easily imagine someone constructing an UDP packet
> that could hash collide with a given TCP flow.
>
> And yes, i40 support matching SCTP, and we will create a
> XDP_HASH_TYPE_L4_SCTP when adding XDP rxhash support for that driver.
>
But where would this information be used? We don't save it in skbuff,
don't use it in RPS, RFS. RSS doesn't use it for packet steering so
the hash collision problem already exists at the device level. If
there is a collision problem between two protocols then maybe hash
should be over 5-tuple instead...

Tom

> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer


Viable partnership request.

2017-05-21 Thread Mr Albert Yang
Compliment of the day,


I have access to very vital information that can be used to move a huge amount 
of money. 
I have done my homework very well and i have the machineries in place to get it 
done. 
Ultimately I need an honest person to play an important role in the completion 
of this business transaction.


Regards,
Albert.



Re: [PATCH net-next v5] net: ipv6: fix code style error and warning of ndisc.c

2017-05-21 Thread Joe Perches
On Sun, 2017-05-21 at 12:54 -0400, David Miller wrote:
> From: David Ahern  Date: Sun, 21 May 2017 10:17:47 -0600
> > On 5/21/17 10:05 AM, Joe Perches wrote:
> >> But really, why bother?
> >> 
> >> Just because checkpatch bleats some message doesn't
> >> mean it _has_ to be fixed.
> >> 
> >> Please strive to make the code more readable and
> >> intelligible for _humans_.  Compilers don't care.
> > 
> > +1
> > 
> > broad cleanups like this make 'git blame' harder to use to find root causes.

I don't have any issue with broad cleanups and I think
the git blame argument is overblown.

If and when broad cleanups are done, it's best if there
is some other concurrent desire to improve logic, reduce
object code size, and/or other code refactoring other
than mere whitespace changes.



[PATCH 5/5] atm: Use seq_putc() in lec_info()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Sun, 21 May 2017 21:50:44 +0200

A single space character should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 net/atm/lec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 752891434074..1d4edf093987 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -796,7 +796,7 @@ static void lec_info(struct seq_file *seq, struct 
lec_arp_table *entry)
 
for (i = 0; i < ETH_ALEN; i++)
seq_printf(seq, "%2.2x", entry->mac_addr[i] & 0xff);
-   seq_printf(seq, " ");
+   seq_putc(seq, ' ');
for (i = 0; i < ATM_ESA_LEN; i++)
seq_printf(seq, "%2.2x", entry->atm_addr[i] & 0xff);
seq_printf(seq, " %s %4.4x", lec_arp_get_status_string(entry->status),
-- 
2.13.0



[PATCH 4/5] atm: Use seq_puts() in lec_info()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Sun, 21 May 2017 21:49:10 +0200

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 net/atm/lec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index ca3aec0c0743..752891434074 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -804,7 +804,7 @@ static void lec_info(struct seq_file *seq, struct 
lec_arp_table *entry)
if (entry->vcc)
seq_printf(seq, "%3d %3d ", entry->vcc->vpi, entry->vcc->vci);
else
-   seq_printf(seq, "");
+   seq_puts(seq, "");
if (entry->recv_vcc) {
seq_printf(seq, " %3d %3d", entry->recv_vcc->vpi,
   entry->recv_vcc->vci);
-- 
2.13.0



[PATCH 3/5] atm: Adjust 19 checks for null pointers

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Sun, 21 May 2017 21:34:23 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 net/atm/lec.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index a11dbd3a5119..ca3aec0c0743 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -139,7 +139,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct 
net_device *dev)
struct atmlec_msg *mesg;
 
skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
-   if (skb2 == NULL)
+   if (!skb2)
return;
skb2->len = sizeof(struct atmlec_msg);
mesg = (struct atmlec_msg *)skb2->data;
@@ -264,7 +264,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
   min_frame_size - skb->truesize,
   GFP_ATOMIC);
dev_kfree_skb(skb);
-   if (skb2 == NULL) {
+   if (!skb2) {
dev->stats.tx_dropped++;
return NETDEV_TX_OK;
}
@@ -431,7 +431,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff 
*skb)
pr_debug("%s: bridge zeppelin asks about %pM\n",
 dev->name, mesg->content.proxy.mac_addr);
 
-   if (br_fdb_test_addr_hook == NULL)
+   if (!br_fdb_test_addr_hook)
break;
 
if (br_fdb_test_addr_hook(dev, mesg->content.proxy.mac_addr)) {
@@ -442,7 +442,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff 
*skb)
pr_debug("%s: entry found, responding to zeppelin\n",
 dev->name);
skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
-   if (skb2 == NULL)
+   if (!skb2)
break;
skb2->len = sizeof(struct atmlec_msg);
skb_copy_to_linear_data(skb2, mesg, sizeof(*mesg));
@@ -520,7 +520,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
mesg = (struct atmlec_msg *)skb->data;
memset(mesg, 0, sizeof(*mesg));
mesg->type = type;
-   if (data != NULL)
+   if (data)
mesg->sizeoftlvs = data->len;
if (mac_addr)
ether_addr_copy(mesg->content.normal.mac_addr, mac_addr);
@@ -534,7 +534,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
skb_queue_tail(&sk->sk_receive_queue, skb);
sk->sk_data_ready(sk);
 
-   if (data != NULL) {
+   if (data) {
pr_debug("about to send %d bytes of data\n", data->len);
atm_force_charge(priv->lecd, data->truesize);
skb_queue_tail(&sk->sk_receive_queue, data);
@@ -663,7 +663,7 @@ static void lec_pop(struct atm_vcc *vcc, struct sk_buff 
*skb)
struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
struct net_device *dev = skb->dev;
 
-   if (vpriv == NULL) {
+   if (!vpriv) {
pr_info("vpriv = NULL!?!?!?\n");
return;
}
@@ -1066,7 +1066,7 @@ static void __exit lane_module_cleanup(void)
deregister_atm_ioctl(&lane_ioctl_ops);
 
for (i = 0; i < MAX_LEC_ITF; i++) {
-   if (dev_lec[i] != NULL) {
+   if (dev_lec[i]) {
unregister_netdev(dev_lec[i]);
free_netdev(dev_lec[i]);
dev_lec[i] = NULL;
@@ -1097,11 +1097,11 @@ static int lane2_resolve(struct net_device *dev, const 
u8 *dst_mac, int force,
spin_lock_irqsave(&priv->lec_arp_lock, flags);
table = lec_arp_find(priv, dst_mac);
spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
-   if (table == NULL)
+   if (!table)
return -1;
 
*tlvs = kmemdup(table->tlvs, table->sizeoftlvs, GFP_ATOMIC);
-   if (*tlvs == NULL)
+   if (!*tlvs)
return -1;
 
*sizeoftlvs = table->sizeoftlvs;
@@ -1109,12 +1109,12 @@ static int lane2_resolve(struct net_device *dev, const 
u8 *dst_mac, int force,
return 0;
}
 
-   if (sizeoftlvs == NULL)
+   if (!sizeoftlvs)
retval = send_to_lecd(priv, l_arp_xmt, dst_mac, NULL, NULL);
 
else {
skb = alloc_skb(*sizeoftlvs, GFP_ATOMIC);
-   if (skb == NULL)
+   if (!skb)
return -1;
  

[PATCH 2/5] atm: Delete an error message for a failed memory allocation in make_entry()

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Sun, 21 May 2017 21:24:45 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 net/atm/lec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 6ad3ca625a44..a11dbd3a5119 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -1556,7 +1556,6 @@ static struct lec_arp_table *make_entry(struct lec_priv 
*priv,
-   if (!to_return) {
-   pr_info("LEC: Arp entry kmalloc failed\n");
+   if (!to_return)
return NULL;
-   }
+
ether_addr_copy(to_return->mac_addr, mac_addr);
INIT_HLIST_NODE(&to_return->next);
setup_timer(&to_return->timer, lec_arp_expire_arp,
-- 
2.13.0



[PATCH 1/5] atm: Improve a size determination in four functions

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Sun, 21 May 2017 21:20:44 +0200

Replace the specification of four data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 net/atm/lec.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 09cfe87f0a44..6ad3ca625a44 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -518,5 +518,5 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
return -1;
skb->len = sizeof(struct atmlec_msg);
mesg = (struct atmlec_msg *)skb->data;
-   memset(mesg, 0, sizeof(struct atmlec_msg));
+   memset(mesg, 0, sizeof(*mesg));
mesg->type = type;
@@ -690,5 +690,5 @@ static int lec_vcc_attach(struct atm_vcc *vcc, void __user 
*arg)
if (ioc_data.dev_num < 0 || ioc_data.dev_num >= MAX_LEC_ITF ||
!dev_lec[ioc_data.dev_num])
return -EINVAL;
-   vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+   vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
if (!vpriv)
@@ -1552,5 +1552,5 @@ static struct lec_arp_table *make_entry(struct lec_priv 
*priv,
 {
struct lec_arp_table *to_return;
 
-   to_return = kzalloc(sizeof(struct lec_arp_table), GFP_ATOMIC);
+   to_return = kzalloc(sizeof(*to_return), GFP_ATOMIC);
if (!to_return) {
@@ -2156,5 +2156,5 @@ static int lec_mcast_make(struct lec_priv *priv, struct 
atm_vcc *vcc)
struct lec_vcc_priv *vpriv;
int err = 0;
 
-   vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+   vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
if (!vpriv)
-- 
2.13.0



[PATCH 0/5] atm: Adjustments for some function implementations

2017-05-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Sun, 21 May 2017 22:09:11 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Improve a size determination in four functions
  Delete an error message for a failed memory allocation in make_entry()
  Adjust 19 checks for null pointers
  Use seq_puts() in lec_info()
  Use seq_putc() in lec_info()

 net/atm/lec.c | 55 +++
 1 file changed, 27 insertions(+), 28 deletions(-)

-- 
2.13.0



Re: [patch net-next 2/2] net/sched: fix filter flushing

2017-05-21 Thread Jiri Pirko
Sun, May 21, 2017 at 08:27:21PM CEST, xiyou.wangc...@gmail.com wrote:
>On Sat, May 20, 2017 at 10:54 PM, Jiri Pirko  wrote:
>> Sun, May 21, 2017 at 02:16:45AM CEST, xiyou.wangc...@gmail.com wrote:
>>>On Sat, May 20, 2017 at 6:01 AM, Jiri Pirko  wrote:
 +static void tcf_chain_destroy(struct tcf_chain *chain)
 +{
 +   list_del(&chain->list);
 +   tcf_chain_flush(chain);
 kfree(chain);
  }

 @@ -510,7 +517,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct 
 nlmsghdr *n,

 if (n->nlmsg_type == RTM_DELTFILTER && prio == 0) {
 tfilter_notify_chain(net, skb, n, chain, RTM_DELTFILTER);
 -   tcf_chain_destroy(chain);
 +   tcf_chain_flush(chain);
>>>
>>>
>>>I wonder if we should return EBUSY and do nothing in case of busy?
>>>The chain is no longer visual to new actions after your list_del(), but
>>>the old one could still use and see it.
>>
>> No. User request to flush the chain, that is what happens in the past
>> and that is what should happen now.
>> If there is still a reference, the chain_put will keep the empty chain.
>
>But if you dump the actions, this chain is still shown "goto chain"?

Yes, it will be shown there.


>You can't claim you really delete it as long as actions can still
>see it and dump it.

No, user just wants to delete all the filters. That is done. User does
not care if the actual chain structure is there or not.



[PATCH net-next] net: socket: fix a typo in sockfd_lookup().

2017-05-21 Thread Rami Rosen
This patch fixes a typo in sockfd_lookup() in net/socket.c.

Signed-off-by: Rami Rosen 
---
 net/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/socket.c b/net/socket.c
index cb355a7..8f9dab3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -461,7 +461,7 @@ EXPORT_SYMBOL(sock_from_file);
  * @err: pointer to an error code return
  *
  * The file handle passed in is locked and the socket it is bound
- * too is returned. If an error occurs the err pointer is overwritten
+ * to is returned. If an error occurs the err pointer is overwritten
  * with a negative errno code and NULL is returned. The function checks
  * for both invalid handles and passing a handle which is not a socket.
  *
-- 
2.7.4



Re: [patch net-next 2/2] net/sched: fix filter flushing

2017-05-21 Thread Cong Wang
On Sat, May 20, 2017 at 10:54 PM, Jiri Pirko  wrote:
> Sun, May 21, 2017 at 02:16:45AM CEST, xiyou.wangc...@gmail.com wrote:
>>On Sat, May 20, 2017 at 6:01 AM, Jiri Pirko  wrote:
>>> +static void tcf_chain_destroy(struct tcf_chain *chain)
>>> +{
>>> +   list_del(&chain->list);
>>> +   tcf_chain_flush(chain);
>>> kfree(chain);
>>>  }
>>>
>>> @@ -510,7 +517,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct 
>>> nlmsghdr *n,
>>>
>>> if (n->nlmsg_type == RTM_DELTFILTER && prio == 0) {
>>> tfilter_notify_chain(net, skb, n, chain, RTM_DELTFILTER);
>>> -   tcf_chain_destroy(chain);
>>> +   tcf_chain_flush(chain);
>>
>>
>>I wonder if we should return EBUSY and do nothing in case of busy?
>>The chain is no longer visual to new actions after your list_del(), but
>>the old one could still use and see it.
>
> No. User request to flush the chain, that is what happens in the past
> and that is what should happen now.
> If there is still a reference, the chain_put will keep the empty chain.

But if you dump the actions, this chain is still shown "goto chain"?
You can't claim you really delete it as long as actions can still
see it and dump it.


Re: [PATCH net-next] tcp: fix tcp_probe_timer() for TCP_USER_TIMEOUT

2017-05-21 Thread Eric Dumazet
On Sun, 2017-05-21 at 13:46 -0400, Soheil Hassas Yeganeh wrote:
> On Sun, May 21, 2017 at 1:39 PM, Eric Dumazet  wrote:
> > From: Eric Dumazet 
> >
> > TCP_USER_TIMEOUT is still converted to jiffies value in
> > icsk_user_timeout
> >
> > So we need to make a conversion for the cases HZ != 1000
> >
> > Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> > Signed-off-by: Eric Dumazet 
> 
> Acked-by: Soheil Hassas Yeganeh 
> 
> Thank you for the fix, Eric!

Thanks for reviewing !




Re: [PATCH net-next] tcp: fix tcp_probe_timer() for TCP_USER_TIMEOUT

2017-05-21 Thread Eric Dumazet
On Sun, 2017-05-21 at 13:51 -0400, David Miller wrote:
> From: Eric Dumazet 
> Date: Sun, 21 May 2017 10:39:00 -0700
> 
> > From: Eric Dumazet 
> > 
> > TCP_USER_TIMEOUT is still converted to jiffies value in
> > icsk_user_timeout
> > 
> > So we need to make a conversion for the cases HZ != 1000 
> > 
> > Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> > Signed-off-by: Eric Dumazet 
> 
> Applied, thank Eric.
> 
> I kinda expected a few pieces of fallout from the 1ms changes :)

Absolutely ;)

One last piece is in TCP_SYNCNT support.

I saw that retransmits_timed_out() could have a rounding error :

tcp_time_stamp(tcp_sk(sk)) - start_ts) ends up to 999 ms,
while the timeout is/was 1000 ms (And timer _was_ progammed with 1000
jiffies for HZ=1000 kernel)

So if user setup TCP_SYNCNT = 1 socket option, we sometime sends one
extra SYN packet.

I will send a fix later.




Re: [PATCH net-next] tcp: fix tcp_probe_timer() for TCP_USER_TIMEOUT

2017-05-21 Thread David Miller
From: Eric Dumazet 
Date: Sun, 21 May 2017 10:39:00 -0700

> From: Eric Dumazet 
> 
> TCP_USER_TIMEOUT is still converted to jiffies value in
> icsk_user_timeout
> 
> So we need to make a conversion for the cases HZ != 1000 
> 
> Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> Signed-off-by: Eric Dumazet 

Applied, thank Eric.

I kinda expected a few pieces of fallout from the 1ms changes :)


Re: [PATCH net-next] tcp: fix tcp_probe_timer() for TCP_USER_TIMEOUT

2017-05-21 Thread Soheil Hassas Yeganeh
On Sun, May 21, 2017 at 1:39 PM, Eric Dumazet  wrote:
> From: Eric Dumazet 
>
> TCP_USER_TIMEOUT is still converted to jiffies value in
> icsk_user_timeout
>
> So we need to make a conversion for the cases HZ != 1000
>
> Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> Signed-off-by: Eric Dumazet 

Acked-by: Soheil Hassas Yeganeh 

Thank you for the fix, Eric!


Re: [PATCH net-next 9/9] ipv6: remove unused variables in esp6

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:56 -0700

> Resolves warnings:
> net/ipv6/esp6.c: In function ‘esp_ssg_unref’:
> net/ipv6/esp6.c:121:10: warning: variable ‘seqhi’ set but not used 
> [-Wunused-but-set-variable]
> net/ipv6/esp6.c: In function ‘esp6_output_head’:
> net/ipv6/esp6.c:227:21: warning: variable ‘esph’ set but not used 
> [-Wunused-but-set-variable]
> 
> Signed-off-by: Stephen Hemminger 

Stephen please always CC: the IPSEC maintainer for IPSEC implementation
fixes.

Steffen, please consider taking this into your tree.

> ---
>  net/ipv6/esp6.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index 1fe99ba8066c..53b6b870b935 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -118,7 +118,6 @@ static inline struct scatterlist *esp_req_sg(struct 
> crypto_aead *aead,
>  
>  static void esp_ssg_unref(struct xfrm_state *x, void *tmp)
>  {
> - __be32 *seqhi;
>   struct crypto_aead *aead = x->data;
>   int seqhilen = 0;
>   u8 *iv;
> @@ -128,7 +127,6 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp)
>   if (x->props.flags & XFRM_STATE_ESN)
>   seqhilen += sizeof(__be32);
>  
> - seqhi = esp_tmp_seqhi(tmp);
>   iv = esp_tmp_iv(aead, tmp, seqhilen);
>   req = esp_tmp_req(aead, iv);
>  
> @@ -224,12 +222,9 @@ int esp6_output_head(struct xfrm_state *x, struct 
> sk_buff *skb, struct esp_info
>   u8 *vaddr;
>   int nfrags;
>   struct page *page;
> - struct ip_esp_hdr *esph;
>   struct sk_buff *trailer;
>   int tailen = esp->tailen;
>  
> - esph = ip_esp_hdr(skb);
> -
>   if (!skb_cloned(skb)) {
>   if (tailen <= skb_availroom(skb)) {
>   nfrags = 1;
> -- 
> 2.11.0
> 


Re: [PATCH net-next 4/9] inet: fix warning about missing prototype

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:51 -0700

> The prototype for inet_rcv_saddr_equal was not being included.
> 
> Signed-off-by: Stephen Hemminger 

Applied.


Re: [PATCH net-next 6/9] xfrm: make xfrm_dev_register static

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:53 -0700

> This function is only used in this file and should not be global.
> 
> Signed-off-by: Stephen Hemminger 

Steffen said he already took this.


Re: [PATCH net-next 7/9] fou: make local function static

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:54 -0700

> The build header functions are not used by any other code.
> 
> net/ipv6/fou6.c:36:5: warning: no previous prototype for ‘fou6_build_header’ 
> [-Wmissing-prototypes]
> net/ipv6/fou6.c:54:5: warning: no previous prototype for ‘gue6_build_header’ 
> [-Wmissing-prototypes]
> 
> Need to do some code rearranging to satisfy different Kconfig possiblities.
> 
> Signed-off-by: Stephen Hemminger 

Applied.


Re: [PATCH net-next 8/9] ipv6: drop unused variables in seg6_genl_dumphac

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:55 -0700

> THe seg6_pernet_data variable was set but never used.
> 
> Signed-off-by: Stephen Hemminger 

Applied.


Re: [PATCH net-next 5/9] tcpnv: do not export local function

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:52 -0700

> The TCP New Vegas congestion control was exporting an internal
> function tcpnv_get_info which is not used by any other in tree
> kernel code. Make it static.
> 
> Signed-off-by: Stephen Hemminger 

Applied.


Re: [PATCH net-next 2/9] ila: propagate error code in ila_output

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:49 -0700

> This warning:
> net/ipv6/ila/ila_lwt.c: In function ‘ila_output’:
> net/ipv6/ila/ila_lwt.c:42:6: warning: variable ‘err’ set but not used 
> [-Wunused-but-set-variable]
> 
> It looks like the code attempts to set propagate different error
> values, but always returned -EINVAL.
> 
> Compile tested only. Needs review by original author.
> 
> Signed-off-by: Stephen Hemminger 

Applied.


Re: [PATCH net-next 1/9] dcb: enforce minimum length on IEEE_APPS attribute

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:48 -0700

> Found by reviewing the warning about unused policy table.
> The code implies that it meant to check for size, but since
> it unrolled the loop for attribute validation that is never used.
> Instead do explicit check for attribute.
> 
> Compile tested only. Needs review by original author.
> 
> Signed-off-by: Stephen Hemminger 

Applied.


Re: [PATCH net-next 3/9] udp: make local function static

2017-05-21 Thread David Miller
From: Stephen Hemminger 
Date: Fri, 19 May 2017 09:55:50 -0700

> udp_queue_rcv_skb was global but only used in one file.
> Identified by this warning:
> net/ipv4/udp.c:1775:5: warning: no previous prototype for ‘udp_queue_rcv_skb’ 
> [-Wmissing-prototypes]
>  int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> 
> Signed-off-by: Stephen Hemminger 

Already fixed in net-next, please generate patches against
current sources.


[PATCH net-next] tcp: fix tcp_probe_timer() for TCP_USER_TIMEOUT

2017-05-21 Thread Eric Dumazet
From: Eric Dumazet 

TCP_USER_TIMEOUT is still converted to jiffies value in
icsk_user_timeout

So we need to make a conversion for the cases HZ != 1000 

Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
Signed-off-by: Eric Dumazet 
---
 net/ipv4/tcp_timer.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 
27a667bce8060e6b2290fe636c27a79d0d593b48..c4a35ba7f8ed0dac573c864900b081b4847927d8
 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -341,7 +341,8 @@ static void tcp_probe_timer(struct sock *sk)
if (!start_ts)
tcp_send_head(sk)->skb_mstamp = tp->tcp_mstamp;
else if (icsk->icsk_user_timeout &&
-(s32)(tcp_time_stamp(tp) - start_ts) > icsk->icsk_user_timeout)
+(s32)(tcp_time_stamp(tp) - start_ts) >
+jiffies_to_msecs(icsk->icsk_user_timeout))
goto abort;
 
max_probes = sock_net(sk)->ipv4.sysctl_tcp_retries2;




Re: [PATCH v6 net-next 0/7] Extend socket timestamping API

2017-05-21 Thread David Miller
From: Miroslav Lichvar 
Date: Fri, 19 May 2017 17:52:34 +0200

> This patchset adds new options to the timestamping API that will be
> useful for NTP implementations and possibly other applications.
> 
> The first patch specifies a timestamp filter for NTP packets. The second
> patch updates drivers that can timestamp all packets, or need to list
> the filter as unsupported. There is no attempt to add the support to the
> phyter driver.
> 
> The third patch adds two helper functions working with NAPI ID, which is
> needed by the next patch. The fourth patch adds a new option to get a
> new control message with the L2 length and interface index for incoming
> packets with hardware timestamps.
> 
> The fifth patch fixes documentation on number of non-zero fields in
> scm_timestamping and warns about false software timestamps when
> SO_TIMESTAMP(NS) is combined with SCM_TIMESTAMPING.
> 
> The sixth patch adds a new option to request both software and hardware
> timestamps for outgoing packets. The seventh patch updates drivers that
> assumed software timestamping cannot be used together with hardware
> timestamping.
> 
> The patches have been tested on x86_64 machines with igb and e1000e
> drivers.

Series applied, thanks.


Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start

2017-05-21 Thread David Miller
From: Xin Long 
Date: Fri, 19 May 2017 22:20:29 +0800

> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
> kernel hello and hold timers"), bridge would not start hello_timer if
> stp_enabled is not KERNEL_STP when br_dev_open.
> 
> The problem is even if users set stp_enabled with KERNEL_STP later,
> the timer will still not be started. It causes that KERNEL_STP can
> not really work. Users have to re-ifup the bridge to avoid this.
> 
> This patch is to fix it by starting br->hello_timer when enabling
> KERNEL_STP in br_stp_start.
> 
> As an improvement, it's also to start hello_timer again only when
> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
> no reason to start the timer again when it's NO_STP.
> 
> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello 
> and hold timers")
> Reported-by: Haidong Li 
> Signed-off-by: Xin Long 

Applied and queued up for -stable, thanks.


Re: [PATCH v2 net] smsc95xx: Support only IPv4 TCP/UDP csum offload

2017-05-21 Thread David Miller
From: 
Date: Fri, 19 May 2017 14:00:25 +

> From: Nisar Sayed 
> 
> When TX checksum offload is used, if the computed checksum is 0 the
> LAN95xx device do not alter the checksum to 0x.  In the case of ipv4
> UDP checksum, it indicates to receiver that no checksum is calculated.
> Under ipv6, UDP checksum yields a result of zero must be changed to
> 0x. Hence disabling checksum offload for ipv6 packets.
> 
> Signed-off-by: Nisar Sayed 
> 
> Reported-by: popcorn mix 

Appied, thanks.


Re: [PATCH net-next] cxgb4 : retrieve port information from firmware

2017-05-21 Thread David Miller
From: Ganesh Goudar 
Date: Fri, 19 May 2017 17:50:15 +0530

> issue get port information command to firmware to retrieve port
> information and update if it is different from what was last
> recorded and also add indication for supported link modes for
> firmware port types FW_PORT_TYPE_SFP28, FW_PORT_TYPE_KR_SFP28,
> FW_PORT_TYPE_CR4_QSFP.
> 
> Based on the original work by Casey Leedom 
> 
> Signed-off-by: Casey Leedom 
> Signed-off-by: Ganesh Goudar 

Applied.


Re: [PATCH net-next v2] ibmveth: Support to enable LSO/CSO for Trunk VEA.

2017-05-21 Thread David Miller
From: Sivakumar Krishnasamy 
Date: Fri, 19 May 2017 05:30:38 -0400

 ...
> Signed-off-by: Sivakumar Krishnasamy 

Applied, thanks for the more detailed commit message.


Re: [PATCH 1/4] net-next: stmmac: Convert new_state to bool

2017-05-21 Thread David Miller
From: Corentin Labbe 
Date: Fri, 19 May 2017 09:03:32 +0200

> This patch convert new_state from int to bool since it store only 1 or 0
> 
> Signed-off-by: Corentin Labbe 

You must also change it to use the values "true" and "false" as well.

Thanks.


Re: [PATCH v2 0/4] arp: always override existing neigh entries with gratuitous ARP

2017-05-21 Thread David Miller
From: Ihar Hrachyshka 
Date: Thu, 18 May 2017 12:41:17 -0700

> This patchset is spurred by discussion started at
> https://patchwork.ozlabs.org/patch/760372/ where we figured that there is no
> real reason for enforcing override by gratuitous ARP packets only when
> arp_accept is 1. Same should happen when it's 0 (the default value).
> 
> changelog v2: handled review comments by Julian Anastasov
> - fixed a mistake in a comment;
> - postponed addr_type calculation to as late as possible.

Series applied, thanks.

Please address the feedback from Julian about the ieee1394 change
you put into ARP earlier.

Thanks.


Re: [PATCH net] tcp: initialize rcv_mss to TCP_MIN_MSS instead of 0

2017-05-21 Thread David Miller
From: Wei Wang 
Date: Thu, 18 May 2017 11:22:33 -0700

> From: Wei Wang 
> 
> When tcp_disconnect() is called, inet_csk_delack_init() sets
> icsk->icsk_ack.rcv_mss to 0.
> This could potentially cause tcp_recvmsg() => tcp_cleanup_rbuf() =>
> __tcp_select_window() call path to have division by 0 issue.
> So this patch initializes rcv_mss to TCP_MIN_MSS instead of 0.
> 
> Reported-by: Andrey Konovalov  
> Signed-off-by: Wei Wang 
> Signed-off-by: Eric Dumazet 
> Signed-off-by: Neal Cardwell 
> Signed-off-by: Yuchung Cheng 

Applied, thanks.


Re: [PATCH net-next 05/13] nfp: introduce very minimal nfp_app

2017-05-21 Thread David Miller
From: Jakub Kicinski 
Date: Fri, 19 May 2017 15:01:47 -0700

> Introduce a concept of an application.  For now it's just grouping
> pointers and serving as a layer of indirection.  It will help us
> weaken the dependency on nfp_net in ethtool code.  Later series
> will flesh out support for different apps in the driver.
> 
> Signed-off-by: Jakub Kicinski 
> Reviewed-by: Simon Horman 
 ...
> +struct nfp_cpp *nfp_app_cpp(struct nfp_app *app)
> +{
> + return app->cpp;
> +}
> +
> +struct nfp_pf *nfp_app_pf(struct nfp_app *app)
> +{
> + return app->pf;
> +}

Don't create real function calls just to dereference pointers
like this.

Instead, if you absolutely _must_ do stuff like this, put it into
inline functions in a header file so that there is no overhead.

But honestly I would just make the core dereference the struct members
directly in the code and do away with these helpers.

Thanks.



Re: [PATCH net-next 00/10] qed/qede updates

2017-05-21 Thread David Miller
From: Yuval Mintz 
Date: Sun, 21 May 2017 12:10:51 +0300

> This series contains some general minor fixes and enhancements:
> 
>  - #1, #2 and #9 correct small missing ethtool functionality.
>  - #3, #6  and #8 correct minor issues in driver, but those are either
>print-related or unexposed in existing code.
>  - #4 adds proper support to TLB mode bonding.
>  - #10 is meant to improve performance on varying cache-line sizes.
> 
> Dave,
> 
> Please consider applying this to `net-next'.

Series applied, thank you.


Re: [PATCH 00/12] Netfilter/IPVS fixes for net

2017-05-21 Thread David Miller
From: Pablo Neira Ayuso 
Date: Fri, 19 May 2017 10:33:41 +0200

> The following patchset contains Netfilter/IPVS fixes for your net tree,
> they are:
 ...
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thanks Pablo.


Re: [PATCH net-next v5] net: ipv6: fix code style error and warning of ndisc.c

2017-05-21 Thread David Miller
From: David Ahern 
Date: Sun, 21 May 2017 10:17:47 -0600

> On 5/21/17 10:05 AM, Joe Perches wrote:
>> But really, why bother?
>> 
>> Just because checkpatch bleats some message doesn't
>> mean it _has_ to be fixed.
>> 
>> Please strive to make the code more readable and
>> intelligible for _humans_.  Compilers don't care.
> 
> +1
> 
> broad cleanups like this make 'git blame' harder to use to find root causes.

I agree, and I'm pretty much going to ignore these ndisc.c cleanup
changes.

Sorry.


brcmfmac firmware issue on NanoPi K2

2017-05-21 Thread Andreas Färber
Hello,

The NanoPi K2 has an Ampak AP6212 SDIO module. brcmfmac driver loads
brcmfmac43430-sdio.bin.

When using the firmware file from linux-firmware.git that openSUSE ships
I get the following errors on 4.11.0 and next-20170519:

[ 2103.618716] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (100):
clkctl 0x50
[ 2104.668746] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (100):
clkctl 0x50
[ 2105.678677] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (100):
clkctl 0x50

If I overwrite /lib/firmware/brcm/bcm43430-sdio.bin with
fw_bcm43438a0.bin from FriendlyARM's Android repository it suddenly works:

[  +0.157738] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0:
Jun  6 2014 14:50:39 version 7.10.226.49 (r) FWID 01-8962686a
[  +0.160108] brcmfmac: brcmf_cfg80211_reg_notifier: not a ISO3166 code
(0x30 0x30)

I recall using the linux-firmware.git brcmfmac43430-sdio.bin file
successfully on the Raspberry Pi 3 with a downstream (Leap 42.2) kernel.

I've tested both nvram_ap6212.txt and nvram_ap6212a.txt, the latter has
the following diff to nvram.txt:

--- nvram_ap6212.txt2017-05-21 04:24:40.372113426 +0200
+++ nvram_ap6212a.txt   2017-05-21 04:24:49.852116599 +0200
@@ -1,4 +1,4 @@
-#AP6212_NVRAM_V1.0_20140603
+#AP6212_NVRAM_V1.0.1_20160606
 # 2.4 GHz, 20 MHz BW mode

 # The following parameter values are just placeholders, need to be updated.
@@ -51,4 +51,4 @@
 muxenab=0x10
 # CLDO PWM voltage settings - 0x4 - 1.1 volt
 #cldo_pwm=0x4
-
+glitch_based_crsmin=1

https://github.com/friendlyarm/android_hardware_amlogic_wifi/tree/l-amlogic-gx-sync/bcm_ampak/config/6212

* Does the linux-firmware.git brcmfmac43430-sdio.bin need a fix for AP6212?
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/brcm

* Does the brcmfmac driver need to distinguish revisions in sdio.c as
done for 43241, plus a separate firmware file?
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0x, 43430),

* Any other ideas?

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH net-next v5] net: ipv6: fix code style error and warning of ndisc.c

2017-05-21 Thread David Ahern
On 5/21/17 10:05 AM, Joe Perches wrote:
> But really, why bother?
> 
> Just because checkpatch bleats some message doesn't
> mean it _has_ to be fixed.
> 
> Please strive to make the code more readable and
> intelligible for _humans_.  Compilers don't care.

+1

broad cleanups like this make 'git blame' harder to use to find root causes.


Re: [net-next] net: ipv6: fix code style error and warning of ndisc.c

2017-05-21 Thread David Ahern
On 5/19/17 10:16 PM, yuan linyu wrote:
> @@ -240,13 +240,15 @@ struct ndisc_options *ndisc_parse_options(const struct 
> net_device *dev,
> "%s: duplicated ND6 option found: 
> type=%d\n",
> __func__, nd_opt->nd_opt_type);
>   } else {
> - ndopts->nd_opt_array[nd_opt->nd_opt_type] = 
> nd_opt;
> + ndopts->nd_opt_array[nd_opt->nd_opt_type] =
> + nd_opt;
>   }
>   break;
>   case ND_OPT_PREFIX_INFO:
>   ndopts->nd_opts_pi_end = nd_opt;
>   if (!ndopts->nd_opt_array[nd_opt->nd_opt_type])
> - ndopts->nd_opt_array[nd_opt->nd_opt_type] = 
> nd_opt;
> + ndopts->nd_opt_array[nd_opt->nd_opt_type] =
> + nd_opt;
>   break;
>  #ifdef CONFIG_IPV6_ROUTE_INFO
>   case ND_OPT_ROUTE_INFO:

This makes the code less readable. Readability needs to trump rigid
80-column coding style.


> @@ -512,7 +513,8 @@ void ndisc_send_na(struct net_device *dev, const struct 
> in6_addr *daddr,
>   in6_ifa_put(ifp);
>   } else {
>   if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
> -
> inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->srcprefs,
> +inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->
> + srcprefs,

again here


I did not finish out this really long patch, but in general the
80-column rule can not be applied so rigidly.


[PATCH net-next 3/4] net: ipv6: Plumb extack through route add functions

2017-05-21 Thread David Ahern
Plumb extack argument down to route add functions.

Signed-off-by: David Ahern 
---
 include/net/ip6_fib.h   |  3 ++-
 include/net/ip6_route.h |  2 +-
 net/ipv6/addrconf.c |  4 ++--
 net/ipv6/ip6_fib.c  | 14 +++-
 net/ipv6/route.c| 57 +++--
 5 files changed, 46 insertions(+), 34 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index c979c878df1c..aa50e2e6fa2a 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -277,7 +277,8 @@ void fib6_clean_all(struct net *net, int (*func)(struct 
rt6_info *, void *arg),
void *arg);
 
 int fib6_add(struct fib6_node *root, struct rt6_info *rt,
-struct nl_info *info, struct mx6_config *mxc);
+struct nl_info *info, struct mx6_config *mxc,
+struct netlink_ext_ack *extack);
 int fib6_del(struct rt6_info *rt, struct nl_info *info);
 
 void inet6_rt_notify(int event, struct rt6_info *rt, struct nl_info *info,
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index f5e625f53367..f3da9dd2a8db 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -90,7 +90,7 @@ void ip6_route_cleanup(void);
 
 int ipv6_route_ioctl(struct net *net, unsigned int cmd, void __user *arg);
 
-int ip6_route_add(struct fib6_config *cfg);
+int ip6_route_add(struct fib6_config *cfg, struct netlink_ext_ack *extack);
 int ip6_ins_rt(struct rt6_info *);
 int ip6_del_rt(struct rt6_info *);
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6a4fb1e629fb..25443fd946a8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2280,7 +2280,7 @@ addrconf_prefix_route(struct in6_addr *pfx, int plen, 
struct net_device *dev,
cfg.fc_flags |= RTF_NONEXTHOP;
 #endif
 
-   ip6_route_add(&cfg);
+   ip6_route_add(&cfg, NULL);
 }
 
 
@@ -2335,7 +2335,7 @@ static void addrconf_add_mroute(struct net_device *dev)
 
ipv6_addr_set(&cfg.fc_dst, htonl(0xFF00), 0, 0, 0);
 
-   ip6_route_add(&cfg);
+   ip6_route_add(&cfg, NULL);
 }
 
 static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index d4bf2c68a545..c1197e167d3e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -473,7 +473,8 @@ static int inet6_dump_fib(struct sk_buff *skb, struct 
netlink_callback *cb)
 static struct fib6_node *fib6_add_1(struct fib6_node *root,
 struct in6_addr *addr, int plen,
 int offset, int allow_create,
-int replace_required, int sernum)
+int replace_required, int sernum,
+struct netlink_ext_ack *extack)
 {
struct fib6_node *fn, *in, *ln;
struct fib6_node *pn = NULL;
@@ -964,7 +965,8 @@ void fib6_force_start_gc(struct net *net)
  */
 
 int fib6_add(struct fib6_node *root, struct rt6_info *rt,
-struct nl_info *info, struct mx6_config *mxc)
+struct nl_info *info, struct mx6_config *mxc,
+struct netlink_ext_ack *extack)
 {
struct fib6_node *fn, *pn = NULL;
int err = -ENOMEM;
@@ -987,7 +989,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
 
fn = fib6_add_1(root, &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
offsetof(struct rt6_info, rt6i_dst), allow_create,
-   replace_required, sernum);
+   replace_required, sernum, extack);
if (IS_ERR(fn)) {
err = PTR_ERR(fn);
fn = NULL;
@@ -1028,7 +1030,8 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
rt->rt6i_src.plen,
offsetof(struct rt6_info, rt6i_src),
-   allow_create, replace_required, sernum);
+   allow_create, replace_required, sernum,
+   extack);
 
if (IS_ERR(sn)) {
/* If it is failed, discard just allocated
@@ -1047,7 +1050,8 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
sn = fib6_add_1(fn->subtree, &rt->rt6i_src.addr,
rt->rt6i_src.plen,
offsetof(struct rt6_info, rt6i_src),
-   allow_create, replace_required, sernum);
+   allow_create, replace_required, sernum,
+   extack);
 
if (IS_ERR(sn)) {
err = PTR_ERR(sn);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index dc61b0b5e64e..ca754ec4054a 100644
--- a/net/ipv6/route.c
+++ b/net/ip

[PATCH net-next 0/4] net: Add extack for route add/delete failures

2017-05-21 Thread David Ahern
Use the extack feature to improve error messages to user on route
add and delete failures.

David Ahern (4):
  net: ipv4: Plumb extack through route add functions
  net: ipv4: Add extack messages for route add failures
  net: ipv6: Plumb extack through route add functions
  net: ipv6: Add extack messages for route add failures

 include/linux/netlink.h  |   5 ++
 include/net/ip6_fib.h|   3 +-
 include/net/ip6_route.h  |   2 +-
 include/net/ip_fib.h |   3 +-
 net/ipv4/fib_frontend.c  |  18 ---
 net/ipv4/fib_lookup.h|   3 +-
 net/ipv4/fib_semantics.c | 133 ---
 net/ipv4/fib_trie.c  |   4 +-
 net/ipv6/addrconf.c  |   4 +-
 net/ipv6/ip6_fib.c   |  18 +--
 net/ipv6/route.c |  97 ++
 11 files changed, 208 insertions(+), 82 deletions(-)

-- 
2.11.0 (Apple Git-81)



[PATCH net-next 2/4] net: ipv4: Add extack messages for route add failures

2017-05-21 Thread David Ahern
Add messages for non-obvious errors (e.g, no need to add text for malloc
failures or ENODEV failures). This mostly covers the annoying EINVAL errors
Some message strings violate the 80-columns but searchable strings need to
trump that rule.

Signed-off-by: David Ahern 
---
 include/linux/netlink.h  |   5 +++
 net/ipv4/fib_frontend.c  |   2 +
 net/ipv4/fib_semantics.c | 115 ++-
 3 files changed, 100 insertions(+), 22 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 5fff5ba5964e..a68aad484c69 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -97,6 +97,11 @@ struct netlink_ext_ack {
 #define NL_SET_ERR_MSG_MOD(extack, msg)\
NL_SET_ERR_MSG((extack), KBUILD_MODNAME ": " msg)
 
+#define NL_SET_BAD_ATTR(extack, attr) do { \
+   if ((extack))   \
+   (extack)->bad_attr = (attr);\
+} while (0)
+
 extern void netlink_kernel_release(struct sock *sk);
 extern int __netlink_change_ngroups(struct sock *sk, unsigned int groups);
 extern int netlink_change_ngroups(struct sock *sk, unsigned int groups);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 511edff76c01..14d2f7bd7c76 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -656,6 +656,7 @@ static int rtm_to_fib_config(struct net *net, struct 
sk_buff *skb,
cfg->fc_nlinfo.nl_net = net;
 
if (cfg->fc_type > RTN_MAX) {
+   NL_SET_ERR_MSG(extack, "Invalid route type");
err = -EINVAL;
goto errout;
}
@@ -726,6 +727,7 @@ static int inet_rtm_delroute(struct sk_buff *skb, struct 
nlmsghdr *nlh,
 
tb = fib_get_table(net, cfg.fc_table);
if (!tb) {
+   NL_SET_ERR_MSG(extack, "FIB table does not exist");
err = -ESRCH;
goto errout;
}
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 8587d1b55b53..4852e183afe0 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -465,7 +466,13 @@ static int fib_count_nexthops(struct rtnexthop *rtnh, int 
remaining,
}
 
/* leftover implies invalid nexthop configuration, discard it */
-   return remaining > 0 ? 0 : nhs;
+   if (remaining > 0) {
+   NL_SET_ERR_MSG(extack,
+  "Invalid nexthop configuration - extra data 
after nexthops");
+   nhs = 0;
+   }
+
+   return nhs;
 }
 
 static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
@@ -477,11 +484,17 @@ static int fib_get_nhs(struct fib_info *fi, struct 
rtnexthop *rtnh,
change_nexthops(fi) {
int attrlen;
 
-   if (!rtnh_ok(rtnh, remaining))
+   if (!rtnh_ok(rtnh, remaining)) {
+   NL_SET_ERR_MSG(extack,
+  "Invalid nexthop configuration - extra 
data after nexthop");
return -EINVAL;
+   }
 
-   if (rtnh->rtnh_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN))
+   if (rtnh->rtnh_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN)) {
+   NL_SET_ERR_MSG(extack,
+  "Invalid flags for nexthop - can not 
contain DEAD or LINKDOWN");
return -EINVAL;
+   }
 
nexthop_nh->nh_flags =
(cfg->fc_flags & ~0xFF) | rtnh->rtnh_flags;
@@ -507,8 +520,12 @@ static int fib_get_nhs(struct fib_info *fi, struct 
rtnexthop *rtnh,
 
nla_entype = nla_find(attrs, attrlen,
  RTA_ENCAP_TYPE);
-   if (!nla_entype)
+   if (!nla_entype) {
+   NL_SET_BAD_ATTR(extack, nla);
+   NL_SET_ERR_MSG(extack,
+  "Encap type is missing");
goto err_inval;
+   }
 
ret = lwtunnel_build_state(nla_get_u16(
   nla_entype),
@@ -729,16 +746,25 @@ static int fib_check_nh(struct fib_config *cfg, struct 
fib_info *fi,
if (nh->nh_flags & RTNH_F_ONLINK) {
unsigned int addr_type;
 
-   if (cfg->fc_scope >= RT_SCOPE_LINK)
+   if (cfg->fc_scope >= RT_SCOPE_LINK) {
+   NL_SET_ERR_MSG(extack,
+  "Nexthop has invalid scope");
return -EINVAL;
+   }
dev = __dev_get_by_index(

[PATCH net-next 1/4] net: ipv4: Plumb extack through route add functions

2017-05-21 Thread David Ahern
Plumb extack argument down to route add functions.

Signed-off-by: David Ahern 
---
 include/net/ip_fib.h |  3 ++-
 net/ipv4/fib_frontend.c  | 16 +---
 net/ipv4/fib_lookup.h|  3 ++-
 net/ipv4/fib_semantics.c | 22 +-
 net/ipv4/fib_trie.c  |  4 ++--
 5 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 6692c5758b33..42e8b8f55f7c 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -263,7 +263,8 @@ struct fib_table {
 
 int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
 struct fib_result *res, int fib_flags);
-int fib_table_insert(struct net *, struct fib_table *, struct fib_config *);
+int fib_table_insert(struct net *, struct fib_table *, struct fib_config *,
+struct netlink_ext_ack *extack);
 int fib_table_delete(struct net *, struct fib_table *, struct fib_config *);
 int fib_table_dump(struct fib_table *table, struct sk_buff *skb,
   struct netlink_callback *cb);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 83e3ed258467..511edff76c01 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -594,7 +594,8 @@ int ip_rt_ioctl(struct net *net, unsigned int cmd, void 
__user *arg)
} else {
tb = fib_new_table(net, cfg.fc_table);
if (tb)
-   err = fib_table_insert(net, tb, &cfg);
+   err = fib_table_insert(net, tb,
+  &cfg, NULL);
else
err = -ENOBUFS;
}
@@ -626,14 +627,15 @@ const struct nla_policy rtm_ipv4_policy[RTA_MAX + 1] = {
 };
 
 static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
-struct nlmsghdr *nlh, struct fib_config *cfg)
+struct nlmsghdr *nlh, struct fib_config *cfg,
+struct netlink_ext_ack *extack)
 {
struct nlattr *attr;
int err, remaining;
struct rtmsg *rtm;
 
err = nlmsg_validate(nlh, sizeof(*rtm), RTA_MAX, rtm_ipv4_policy,
-NULL);
+extack);
if (err < 0)
goto errout;
 
@@ -718,7 +720,7 @@ static int inet_rtm_delroute(struct sk_buff *skb, struct 
nlmsghdr *nlh,
struct fib_table *tb;
int err;
 
-   err = rtm_to_fib_config(net, skb, nlh, &cfg);
+   err = rtm_to_fib_config(net, skb, nlh, &cfg, extack);
if (err < 0)
goto errout;
 
@@ -741,7 +743,7 @@ static int inet_rtm_newroute(struct sk_buff *skb, struct 
nlmsghdr *nlh,
struct fib_table *tb;
int err;
 
-   err = rtm_to_fib_config(net, skb, nlh, &cfg);
+   err = rtm_to_fib_config(net, skb, nlh, &cfg, extack);
if (err < 0)
goto errout;
 
@@ -751,7 +753,7 @@ static int inet_rtm_newroute(struct sk_buff *skb, struct 
nlmsghdr *nlh,
goto errout;
}
 
-   err = fib_table_insert(net, tb, &cfg);
+   err = fib_table_insert(net, tb, &cfg, extack);
 errout:
return err;
 }
@@ -845,7 +847,7 @@ static void fib_magic(int cmd, int type, __be32 dst, int 
dst_len, struct in_ifad
cfg.fc_scope = RT_SCOPE_HOST;
 
if (cmd == RTM_NEWROUTE)
-   fib_table_insert(net, tb, &cfg);
+   fib_table_insert(net, tb, &cfg, NULL);
else
fib_table_delete(net, tb, &cfg);
 }
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index 9c02920725db..2704e08545da 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -28,7 +28,8 @@ static inline void fib_alias_accessed(struct fib_alias *fa)
 
 /* Exported by fib_semantics.c */
 void fib_release_info(struct fib_info *);
-struct fib_info *fib_create_info(struct fib_config *cfg);
+struct fib_info *fib_create_info(struct fib_config *cfg,
+struct netlink_ext_ack *extack);
 int fib_nh_match(struct fib_config *cfg, struct fib_info *fi);
 int fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event, u32 tb_id,
  u8 type, __be32 dst, int dst_len, u8 tos, struct fib_info *fi,
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index da449ddb8cc1..8587d1b55b53 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -454,7 +454,8 @@ static int fib_detect_death(struct fib_info *fi, int order,
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 
-static int fib_count_nexthops(struct rtnexthop *rtnh, int remaining)
+static int fib_count_nexthops(struct rtnexthop *rtnh, int remaining,
+ struct netlink_ext_ack *extack)
 {
int nhs = 0;
 
@@ -468,7 +469,8 @@ static int fib_count_nexthops(struct rt

[PATCH net-next 4/4] net: ipv6: Add extack messages for route add failures

2017-05-21 Thread David Ahern
Add messages for non-obvious errors (e.g, no need to add text for malloc
failures or ENODEV failures). This mostly covers the annoying EINVAL errors
Some message strings violate the 80-columns but searchable strings need to
trump that rule.

Signed-off-by: David Ahern 
---
 net/ipv6/ip6_fib.c |  4 
 net/ipv6/route.c   | 40 
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index c1197e167d3e..deea901746c8 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -498,6 +498,8 @@ static struct fib6_node *fib6_add_1(struct fib6_node *root,
!ipv6_prefix_equal(&key->addr, addr, fn->fn_bit)) {
if (!allow_create) {
if (replace_required) {
+   NL_SET_ERR_MSG(extack,
+  "Can not replace route - 
no match found");
pr_warn("Can't replace route, no match 
found\n");
return ERR_PTR(-ENOENT);
}
@@ -544,6 +546,8 @@ static struct fib6_node *fib6_add_1(struct fib6_node *root,
 * That would keep IPv6 consistent with IPv4
 */
if (replace_required) {
+   NL_SET_ERR_MSG(extack,
+  "Can not replace route - no match 
found");
pr_warn("Can't replace route, no match found\n");
return ERR_PTR(-ENOENT);
}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ca754ec4054a..80bda31ffbbe 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1857,14 +1857,25 @@ static struct rt6_info *ip6_route_info_create(struct 
fib6_config *cfg,
int err = -EINVAL;
 
/* RTF_PCPU is an internal flag; can not be set by userspace */
-   if (cfg->fc_flags & RTF_PCPU)
+   if (cfg->fc_flags & RTF_PCPU) {
+   NL_SET_ERR_MSG(extack, "Userspace can not set RTF_PCPU");
goto out;
+   }
 
-   if (cfg->fc_dst_len > 128 || cfg->fc_src_len > 128)
+   if (cfg->fc_dst_len > 128) {
+   NL_SET_ERR_MSG(extack, "Invalid prefix length");
+   goto out;
+   }
+   if (cfg->fc_src_len > 128) {
+   NL_SET_ERR_MSG(extack, "Invalid source address length");
goto out;
+   }
 #ifndef CONFIG_IPV6_SUBTREES
-   if (cfg->fc_src_len)
+   if (cfg->fc_src_len) {
+   NL_SET_ERR_MSG(extack,
+  "Specifying source address requires 
IPV6_SUBTREES to be enabled");
goto out;
+   }
 #endif
if (cfg->fc_ifindex) {
err = -ENODEV;
@@ -2015,9 +2026,10 @@ static struct rt6_info *ip6_route_info_create(struct 
fib6_config *cfg,
err = -EINVAL;
if (ipv6_chk_addr_and_flags(net, gw_addr,
gwa_type & IPV6_ADDR_LINKLOCAL ?
-   dev : NULL, 0, 0))
+   dev : NULL, 0, 0)) {
+   NL_SET_ERR_MSG(extack, "Invalid gateway address");
goto out;
-
+   }
rt->rt6i_gateway = *gw_addr;
 
if (gwa_type != (IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST)) {
@@ -2033,8 +2045,11 @@ static struct rt6_info *ip6_route_info_create(struct 
fib6_config *cfg,
   addressing
 */
if (!(gwa_type & (IPV6_ADDR_UNICAST |
- IPV6_ADDR_MAPPED)))
+ IPV6_ADDR_MAPPED))) {
+   NL_SET_ERR_MSG(extack,
+  "Invalid gateway address");
goto out;
+   }
 
if (cfg->fc_table) {
grt = ip6_nh_lookup_table(net, cfg, gw_addr);
@@ -2074,8 +2089,14 @@ static struct rt6_info *ip6_route_info_create(struct 
fib6_config *cfg,
goto out;
}
err = -EINVAL;
-   if (!dev || (dev->flags & IFF_LOOPBACK))
+   if (!dev) {
+   NL_SET_ERR_MSG(extack, "Egress device not specified");
+   goto out;
+   } else if (dev->flags & IFF_LOOPBACK) {
+   NL_SET_ERR_MSG(extack,
+  "Egress device can not be loopback 
device for this route");
goto out;
+   }
}
 
err = -ENODEV;
@@ -2084,6 +2105,7 @@ static struct rt6_info *ip6_route_info_create(struct 
fib6_config *cfg,
 
if (!ipv6_addr_any(&cfg->fc_prefsrc)) {
if (!ipv6_c

Re: [PATCH net-next v5] net: ipv6: fix code style error and warning of ndisc.c

2017-05-21 Thread Joe Perches
On Sun, 2017-05-21 at 08:55 +0800, yuan linyu wrote:
> From: yuan linyu 
> 
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
[]
> @@ -512,7 +519,8 @@ void ndisc_send_na(struct net_device *dev, const struct 
> in6_addr *daddr,
>   in6_ifa_put(ifp);
>   } else {
>   if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
> -
> inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->srcprefs,
> +inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->
> + srcprefs,

This is not a good change as it puts a single dereference
on multiple lines.

> @@ -896,20 +910,19 @@ static void ndisc_recv_ns(struct sk_buff *skb)
>   else
>   NEIGH_CACHE_STAT_INC(&nd_tbl, rcv_probes_ucast);
>  
> - /*
> -  *  update / create cache entry
> + /*  update / create cache entry
>*  for the source address

Some of these comments could be single line

> @@ -1003,30 +1016,31 @@ static void ndisc_recv_na(struct sk_buff *skb)
[]
>   ndisc_update(dev, neigh, lladdr,
> -  msg->icmph.icmp6_solicited ? NUD_REACHABLE : 
> NUD_STALE,
> -  NEIGH_UPDATE_F_WEAK_OVERRIDE|
> -  (msg->icmph.icmp6_override ? 
> NEIGH_UPDATE_F_OVERRIDE : 0)|
> -  NEIGH_UPDATE_F_OVERRIDE_ISROUTER|
> -  (msg->icmph.icmp6_router ? NEIGH_UPDATE_F_ISROUTER 
> : 0),
> +  msg->icmph.icmp6_solicited ?
> + NUD_REACHABLE : NUD_STALE,
> +  NEIGH_UPDATE_F_WEAK_OVERRIDE |
> +  (msg->icmph.icmp6_override ?
> + NEIGH_UPDATE_F_OVERRIDE : 0) |
> +  NEIGH_UPDATE_F_OVERRIDE_ISROUTER |
> +  (msg->icmph.icmp6_router ?
> + NEIGH_UPDATE_F_ISROUTER : 0),
>NDISC_NEIGHBOUR_ADVERTISEMENT, &ndopts);

This is much more difficult to read now and could
be slightly improved by a temporary for msg->icmph,
removing unnecessary parentheses or using multiple
line tests for the flags argument of ndisc_update
instead of the slightly difficult to read uses of
multiple ternaries with bitwise ORs.

Something like:

struct icmp6hdr *icmph = &msg->icmph;
u32 flags;
[...]
flags = NEIGH_UPDATE_F_WEAK_OVERRIDE | 
NEIGH_UPDATE_F_OVERRIDE_ISROUTER;
if (icmph->icmp6_override)
flags |= NEIGH_UPDATE_F_OVERRIDE;
if (icmph->icmp6_router)
flags |= NEIGH_UPDATE_F_ISROUTER;

ndisc_update(dev, neigh, lladdr,
 icmph->icmp6_solicited ? NUD_REACHABLE : NUD_STALE,
 flags, NDISC_NEIGHBOUR_ADVERTISEMENT, &ndopts);

But really, why bother?

Just because checkpatch bleats some message doesn't
mean it _has_ to be fixed.

Please strive to make the code more readable and
intelligible for _humans_.  Compilers don't care.



Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers

2017-05-21 Thread Jesper Dangaard Brouer
On Sat, 20 May 2017 09:16:09 -0700
Tom Herbert  wrote:

> > +/* XDP rxhash have an associated type, which is related to the RSS
> > + * (Receive Side Scaling) standard, but NIC HW have different mapping
> > + * and support. Thus, create mapping that is interesting for XDP.  XDP
> > + * would primarly want insight into L3 and L4 protocol info.
> > + *
> > + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
> > + *
> > + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
> > + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
> > + */
> > +#define XDP_HASH(x)((x) & ((1ULL << 32)-1))
> > +#define XDP_HASH_TYPE(x)   ((x) >> 32)
> > +
> > +#define XDP_HASH_TYPE_L3_SHIFT 0
> > +#define XDP_HASH_TYPE_L3_BITS  3
> > +#define XDP_HASH_TYPE_L3_MASK  ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
> > +#define XDP_HASH_TYPE_L3(x)((x) & XDP_HASH_TYPE_L3_MASK)
> > +enum {
> > +   XDP_HASH_TYPE_L3_IPV4 = 1,
> > +   XDP_HASH_TYPE_L3_IPV6,
> > +};
> > +
> > +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
> > +#define XDP_HASH_TYPE_L4_BITS  5
> > +#define XDP_HASH_TYPE_L4_MASK  \
> > +   (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
> > +#define XDP_HASH_TYPE_L4(x)((x) & XDP_HASH_TYPE_L4_MASK)
> > +enum {
> > +   _XDP_HASH_TYPE_L4_TCP = 1,
> > +   _XDP_HASH_TYPE_L4_UDP,
> > +};
> > +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << 
> > XDP_HASH_TYPE_L4_SHIFT)
> > +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << 
> > XDP_HASH_TYPE_L4_SHIFT)
> > +  
> Hi Jesper,
> 
> Why do we need these indicators for protocol specific hash? It seems
> like L4 and L3 is useful differentiation and protocol agnostic (I'm
> still holding out hope that SCTP will be deployed some day ;-) )

I'm not sure I understood the question fully, but let me try to answer
anyway.  To me it seems obvious that you would want to know the
protocol/L4 type, as this helps avoid hash collisions between UDP and
TCP flows.  I can easily imagine someone constructing an UDP packet
that could hash collide with a given TCP flow.

And yes, i40 support matching SCTP, and we will create a
XDP_HASH_TYPE_L4_SCTP when adding XDP rxhash support for that driver.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers

2017-05-21 Thread Jesper Dangaard Brouer
On Fri, 19 May 2017 20:07:52 -0700
Alexei Starovoitov  wrote:

> On Thu, May 18, 2017 at 05:41:48PM +0200, Jesper Dangaard Brouer wrote:
> >  
> > +/* XDP rxhash have an associated type, which is related to the RSS
> > + * (Receive Side Scaling) standard, but NIC HW have different mapping
> > + * and support. Thus, create mapping that is interesting for XDP.  XDP
> > + * would primarly want insight into L3 and L4 protocol info.
> > + *
> > + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
> > + *
> > + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
> > + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
> > + */
> > +#define XDP_HASH(x)((x) & ((1ULL << 32)-1))
> > +#define XDP_HASH_TYPE(x)   ((x) >> 32)
> > +
> > +#define XDP_HASH_TYPE_L3_SHIFT 0
> > +#define XDP_HASH_TYPE_L3_BITS  3
> > +#define XDP_HASH_TYPE_L3_MASK  ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
> > +#define XDP_HASH_TYPE_L3(x)((x) & XDP_HASH_TYPE_L3_MASK)
> > +enum {
> > +   XDP_HASH_TYPE_L3_IPV4 = 1,
> > +   XDP_HASH_TYPE_L3_IPV6,
> > +};
> > +
> > +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
> > +#define XDP_HASH_TYPE_L4_BITS  5
> > +#define XDP_HASH_TYPE_L4_MASK  
> > \
> > +   (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
> > +#define XDP_HASH_TYPE_L4(x)((x) & XDP_HASH_TYPE_L4_MASK)
> > +enum {
> > +   _XDP_HASH_TYPE_L4_TCP = 1,
> > +   _XDP_HASH_TYPE_L4_UDP,
> > +};
> > +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << 
> > XDP_HASH_TYPE_L4_SHIFT)
> > +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << 
> > XDP_HASH_TYPE_L4_SHIFT)  
> 
> imo this is dangerous territory.
> As far as I can see this information doesn't exist in the current drivers at 
> all
> and you're enabling it in the patch 5 via fancy:
> +   u32 ht = (mlx5_htype_l4_to_xdp[((cht & CQE_RSS_HTYPE_L4) >> 6)] | \
> + mlx5_htype_l3_to_xdp[((cht & CQE_RSS_HTYPE_IP) >> 2)]);
> 
> It's pretty cool that you've discovered this hidden mlx5 feature
> Did you find it in some hw spec ?

The Mellanox ConnectX-4/mlx5 spec is actually open, see:
[1] 
http://www.mellanox.com/page/products_dyn?product_family=204&mtag=connectx_4_en_card
and follow link to "Programming Manual (PRM)".


> And it looks useful to me, but

> 1. i'm worried that we'd be relying on something that mellanox didn't
>  implement in their drivers before. Was it tested and guarnteed to
>  exist in the future revisions of firmware? Is it cx4 or cx4-lx or cx5
>  feature?

It is not a hidden mlx5 or specific feature.  Due to the Microsoft RSS
standard/requirements[2] most NICs actually implement this.

[2] 
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-hashing-types


> 2. but the main concern that it is mellanox only feature. At least I cannot
> see anything like this in broadcom and intel nics

All the drivers I looked at have support for an RSS hash type.
Including Broadcom[3] and Intel. Just grep after NETIF_F_RXHASH, and
follow data-structs.  The Intel i40 NIC have the most elaborate rss type
system (it can e.g. tell if this was SCTP).

[3] 
http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h#L4198
 
> In the very beginning we discussed that XDP programs should be as
> generic as possible and HW independent while at the same time we want
> to expose HW specific features to XDP programs.
>
> So I'm totally fine to expose this fancy hw hash and ipv4 vs v6 and
> tcp vs udp flags to xdp programs somehow, but I'm completely against
> making it into uapi.
> 
> How about exposing 'struct mlx5_cqe64 *' to XDP programs as-is?
> We can make sure that XDP program does read only access into it and
> it will see cqe->rss_hash_result, cqe->rss_hash_type and everything else
> in there, but this will not be uapi and it will be pretty obvious
> to program authors that their programs are vendor specific.

This sounds EXTREMELY dangerous to me... IHMO this will lead to vendor
lock-in.  As BPF program authors will become dependent on vendor
specific features, and their program are no longer portable to run on
other NICs.

How are you going to avoid vendor lock-in with this model?


> 'not uapi' here means that mellanox is free to change their HW descriptor
> and its contents as they wish.

Hmmm... IMHO directly exposing the HW descriptor to userspace, will
limit vendors ability to change its contents.


> Also no copies and bit conversions will be necessary, so the cost will
> be zero to programs that don't use it and we wouldn't need to change
> verifier to discover access to this stuff.

I'm not sure this would work out well, as we would need to keep the
CQE descriptor memory around longer.


The longer term plan with having RXHASH for XDP is to allow
implementing RPS (Receive Packet Steering) without touching packet
memory.  Which plays into my plans for X

Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example

2017-05-21 Thread Andreas Färber
Am 15.05.2017 um 22:13 schrieb Martin Blumenstingl:
> The example in the BCM43xx documentation uses "brcmf" as node name.

No, it doesn't, it uses "bcrmf".

This typo has spread to all ARM device trees:

$ git grep bcrmf -- arch/arm/boot/dts/
arch/arm/boot/dts/imx6sx-nitrogen6sx.dts:   brcmf: bcrmf@1 {
arch/arm/boot/dts/imx6ul-opos6ul.dtsi:  brcmf: bcrmf@1 {
arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts:   brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-bananapi-m1-plus.dts:   brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-bananapro.dts:  brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-cubietruck.dts: brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-i12-tvbox.dts:  brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-wits-pro-a20-dkt.dts:   brcmf: bcrmf@1 {
arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts:brcmf: bcrmf@1 {

For arch/arm64/boot/dts/amlogic/* I've already fixed it, originally by
changing to "brcmf", in a second step "wifi" was requested.

> However, wireless devices should be named "wifi" instead. Fix this to
> make sure that .dts authors can simply use the documentation as
> reference (or simply copy the node from the documentation and then
> adjust only the board specific bits).
> 
> Signed-off-by: Martin Blumenstingl 
> ---
>  Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt 
> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> index 5dbf169cd81c..590f622188de 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> @@ -31,7 +31,7 @@ mmc3: mmc@01c12000 {
>   non-removable;
>   status = "okay";
>  
> - brcmf: bcrmf@1 {
> + brcmf: wifi@1 {
>   reg = <1>;
>   compatible = "brcm,bcm4329-fmac";
>   interrupt-parent = <&pio>;

Thanks for fixing this at its source.

Hopefully the maintainers in CC can make sure that at least it doesn't
spread further into new DTs.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example

2017-05-21 Thread Andreas Färber
Hi,

Am 16.05.2017 um 21:56 schrieb Martin Blumenstingl:
> On Tue, May 16, 2017 at 12:05 AM, Arend Van Spriel
>  wrote:
>> On 15-5-2017 22:13, Martin Blumenstingl wrote:
>>> The example in the BCM43xx documentation uses "brcmf" as node name.
>>> However, wireless devices should be named "wifi" instead. Fix this to
>>
>> Since when is that a rule. I never got the memo and the DTC did not ever
>> complain to me about the naming.

How do you expect it to? Maintain a blacklist of every device model
someone might use, including all typo variations?

> That being said I do not really care
>> and I suppose it is for the sake of consistency only.
> I'm not sure if it's actually a rule or (as you already noted) just
> for consistency. back when I added devicetree support to ath9k Rob
> pointed out that the node should be named "wifi" (instead of "ath9k"),
> see [0]

The general rule is that the node name should be the type of the device,
not duplicate its compatible string.

For consistency Rob was asking we use "wifi" as node name.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)


[PATCH net-next 06/10] qed: Correct print in iscsi error-flow

2017-05-21 Thread Yuval Mintz
If too many CQs are requested, qed would print the available
number as if it's a resource and not a feature leading to the
wrong print.

Fixes: 08737a3fa30a ("qed: Inform qedi the number of possible CQs")
Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c 
b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
index ac3c692..38488db 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
@@ -186,7 +186,7 @@ struct qed_iscsi_conn {
DP_ERR(p_hwfn,
   "Cannot satisfy CQ amount. Queues requested %d, CQs 
available %d. Aborting function start\n",
   p_params->num_queues,
-  p_hwfn->hw_info.resc_num[QED_ISCSI_CQ]);
+  p_hwfn->hw_info.feat_num[QED_ISCSI_CQ]);
return -EINVAL;
}
 
-- 
1.9.3



[PATCH net-next 08/10] qed: Fix setting of Management bitfields

2017-05-21 Thread Yuval Mintz
From: Tomer Tayar 

The management firmware HSI contains masks which are already
shifted to their right place, so QED_MFW_SET_FIELD() is clearing
incorrect fields by shifting the mask by the offset.

Luckily, today we set the fields in an incrementing order [so we're
not erasing any previously set fields], but this still needs fixing.

Signed-off-by: Tomer Tayar 
Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qed/qed.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed.h 
b/drivers/net/ethernet/qlogic/qed/qed.h
index 162cd7f..fd8cf31 100644
--- a/drivers/net/ethernet/qlogic/qed/qed.h
+++ b/drivers/net/ethernet/qlogic/qed/qed.h
@@ -92,7 +92,7 @@ enum qed_coalescing_mode {
 
 #define QED_MFW_SET_FIELD(name, field, value) \
do {   \
-   (name)  &= ~((field ## _MASK) << (field ## _SHIFT));   \
+   (name)  &= ~(field ## _MASK);  \
(name)  |= (((value) << (field ## _SHIFT)) & (field ## _MASK));\
} while (0)
 
-- 
1.9.3



[PATCH net-next 07/10] qede: qedr closure after setting state

2017-05-21 Thread Yuval Mintz
This is benign, but it makes more sense to start the close sequence
only after changing the internal state [in case it would once care].

Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qede/qede_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c 
b/drivers/net/ethernet/qlogic/qede/qede_main.c
index a66bdfe..f0871e1 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -1899,9 +1899,10 @@ static void qede_unload(struct qede_dev *edev, enum 
qede_unload_mode mode,
if (!is_locked)
__qede_lock(edev);
 
-   qede_roce_dev_event_close(edev);
edev->state = QEDE_STATE_CLOSED;
 
+   qede_roce_dev_event_close(edev);
+
/* Close OS Tx */
netif_tx_disable(edev->ndev);
netif_carrier_off(edev->ndev);
-- 
1.9.3



[PATCH net-next 05/10] qed: Revise alloc/setup/free flow

2017-05-21 Thread Yuval Mintz
From: Tomer Tayar 

Re-organize the logic that allocates and frees memory of various
sub-components of the hw-function -

 a. No need to pass pointers to said structure as parameters;
The internal logic knows exactly where to find/set the data.

 b. Nullify pointers after cleanup to prevent possible errors to
re-entrant code.

Signed-off-by: Tomer Tayar 
Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qed/qed_dcbx.c |  1 +
 drivers/net/ethernet/qlogic/qed/qed_dev.c  | 79 +++---
 drivers/net/ethernet/qlogic/qed/qed_fcoe.c | 23 
 drivers/net/ethernet/qlogic/qed/qed_fcoe.h | 22 +++
 drivers/net/ethernet/qlogic/qed/qed_init_ops.c |  4 ++
 drivers/net/ethernet/qlogic/qed/qed_int.c  |  3 +
 drivers/net/ethernet/qlogic/qed/qed_iscsi.c| 22 ---
 drivers/net/ethernet/qlogic/qed/qed_iscsi.h| 23 
 drivers/net/ethernet/qlogic/qed/qed_ll2.c  | 21 ---
 drivers/net/ethernet/qlogic/qed/qed_ll2.h  | 13 ++---
 drivers/net/ethernet/qlogic/qed/qed_mcp.c  |  1 +
 drivers/net/ethernet/qlogic/qed/qed_ooo.c  | 30 ++
 drivers/net/ethernet/qlogic/qed/qed_ooo.h  | 26 -
 drivers/net/ethernet/qlogic/qed/qed_sp.h   | 32 ---
 drivers/net/ethernet/qlogic/qed/qed_spq.c  | 54 ++
 15 files changed, 177 insertions(+), 177 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c 
b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
index b7ca0e2..b83fe1d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
@@ -923,6 +923,7 @@ int qed_dcbx_info_alloc(struct qed_hwfn *p_hwfn)
 void qed_dcbx_info_free(struct qed_hwfn *p_hwfn)
 {
kfree(p_hwfn->p_dcbx_info);
+   p_hwfn->p_dcbx_info = NULL;
 }
 
 static void qed_dcbx_update_protocol_data(struct protocol_dcb_data *p_data,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c 
b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 463927f..3fc3b2e 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -161,6 +161,7 @@ void qed_resc_free(struct qed_dev *cdev)
cdev->fw_data = NULL;
 
kfree(cdev->reset_stats);
+   cdev->reset_stats = NULL;
 
for_each_hwfn(cdev, i) {
struct qed_hwfn *p_hwfn = &cdev->hwfns[i];
@@ -168,18 +169,18 @@ void qed_resc_free(struct qed_dev *cdev)
qed_cxt_mngr_free(p_hwfn);
qed_qm_info_free(p_hwfn);
qed_spq_free(p_hwfn);
-   qed_eq_free(p_hwfn, p_hwfn->p_eq);
-   qed_consq_free(p_hwfn, p_hwfn->p_consq);
+   qed_eq_free(p_hwfn);
+   qed_consq_free(p_hwfn);
qed_int_free(p_hwfn);
 #ifdef CONFIG_QED_LL2
-   qed_ll2_free(p_hwfn, p_hwfn->p_ll2_info);
+   qed_ll2_free(p_hwfn);
 #endif
if (p_hwfn->hw_info.personality == QED_PCI_FCOE)
-   qed_fcoe_free(p_hwfn, p_hwfn->p_fcoe_info);
+   qed_fcoe_free(p_hwfn);
 
if (p_hwfn->hw_info.personality == QED_PCI_ISCSI) {
-   qed_iscsi_free(p_hwfn, p_hwfn->p_iscsi_info);
-   qed_ooo_free(p_hwfn, p_hwfn->p_ooo_info);
+   qed_iscsi_free(p_hwfn);
+   qed_ooo_free(p_hwfn);
}
qed_iov_free(p_hwfn);
qed_dmae_info_free(p_hwfn);
@@ -843,15 +844,7 @@ static int qed_alloc_qm_data(struct qed_hwfn *p_hwfn)
 
 int qed_resc_alloc(struct qed_dev *cdev)
 {
-   struct qed_iscsi_info *p_iscsi_info;
-   struct qed_fcoe_info *p_fcoe_info;
-   struct qed_ooo_info *p_ooo_info;
-#ifdef CONFIG_QED_LL2
-   struct qed_ll2_info *p_ll2_info;
-#endif
u32 rdma_tasks, excess_tasks;
-   struct qed_consq *p_consq;
-   struct qed_eq *p_eq;
u32 line_count;
int i, rc = 0;
 
@@ -956,45 +949,38 @@ int qed_resc_alloc(struct qed_dev *cdev)
DP_ERR(p_hwfn,
   "Cannot allocate 0x%x EQ elements. The maximum 
of a u16 chain is 0x%x\n",
   n_eqes, 0x);
-   rc = -EINVAL;
-   goto alloc_err;
+   goto alloc_no_mem;
}
 
-   p_eq = qed_eq_alloc(p_hwfn, (u16) n_eqes);
-   if (!p_eq)
-   goto alloc_no_mem;
-   p_hwfn->p_eq = p_eq;
+   rc = qed_eq_alloc(p_hwfn, (u16) n_eqes);
+   if (rc)
+   goto alloc_err;
 
-   p_consq = qed_consq_alloc(p_hwfn);
-   if (!p_consq)
-   goto alloc_no_mem;
-   p_hwfn->p_consq = p_consq;
+   rc = qed_consq_alloc(p_hwfn);
+   if (rc)
+   goto alloc_err;
 
 #ifdef CONFIG_QED_LL2
if (p_hwfn->using_ll2) {
-

[PATCH net-next 09/10] qede: Support 1G advertisment.

2017-05-21 Thread Yuval Mintz
From: Sudarsana Reddy Kalluru 

Some variants of adapters support the 1G speed capability. Need to
allow the configuration of 1G speed if adapter supports it.

Signed-off-by: Sudarsana Reddy Kalluru 
Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c 
b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 47ec4f3..6c76a12 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -506,6 +506,14 @@ static int qede_set_link_ksettings(struct net_device *dev,
params.autoneg = false;
params.forced_speed = base->speed;
switch (base->speed) {
+   case SPEED_1000:
+   if (!(current_link.supported_caps &
+ QED_LM_1000baseT_Full_BIT)) {
+   DP_INFO(edev, "1G speed not supported\n");
+   return -EINVAL;
+   }
+   params.adv_speeds = QED_LM_1000baseT_Full_BIT;
+   break;
case SPEED_1:
if (!(current_link.supported_caps &
  QED_LM_1baseKR_Full_BIT)) {
-- 
1.9.3



[PATCH net-next 03/10] qede: Add missing Status-block free

2017-05-21 Thread Yuval Mintz
From: Sudarsana Reddy Kalluru 

When destroying the datapath channels, qede doesn't notify qed of the
released status blocks which were acquired during the initialization.

Signed-off-by: Sudarsana Reddy Kalluru 
Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qede/qede_main.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c 
b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 766bd37..aea9dcf 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -1072,12 +1072,15 @@ static int qede_set_num_queues(struct qede_dev *edev)
return rc;
 }
 
-static void qede_free_mem_sb(struct qede_dev *edev,
-struct qed_sb_info *sb_info)
+static void qede_free_mem_sb(struct qede_dev *edev, struct qed_sb_info 
*sb_info,
+u16 sb_id)
 {
-   if (sb_info->sb_virt)
+   if (sb_info->sb_virt) {
+   edev->ops->common->sb_release(edev->cdev, sb_info, sb_id);
dma_free_coherent(&edev->pdev->dev, sizeof(*sb_info->sb_virt),
  (void *)sb_info->sb_virt, sb_info->sb_phys);
+   memset(sb_info, 0, sizeof(*sb_info));
+   }
 }
 
 /* This function allocates fast-path status block memory */
@@ -1334,7 +1337,7 @@ static int qede_alloc_mem_txq(struct qede_dev *edev, 
struct qede_tx_queue *txq)
 /* This function frees all memory of a single fp */
 static void qede_free_mem_fp(struct qede_dev *edev, struct qede_fastpath *fp)
 {
-   qede_free_mem_sb(edev, fp->sb_info);
+   qede_free_mem_sb(edev, fp->sb_info, fp->id);
 
if (fp->type & QEDE_FASTPATH_RX)
qede_free_mem_rxq(edev, fp->rxq);
-- 
1.9.3



[PATCH net-next 04/10] qede: Don't use an internal MAC field

2017-05-21 Thread Yuval Mintz
Driver maintains its primary MAC in a private field which
gets updated when ndo_dev_set_mac() gets called.

However, there are flows where the primary MAC of the device can change
without said NDO being called [bond device in TLB mode configuring
slaves' addresses], resulting in a configuration where there's a mismatch
between what's apparent to user [the netdevice's value] and what's
configured in the HW [the private value].

As we don't have any real motivation of maintaining this
private field, simply remove it and start using the netdevice's
field instead.

Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qede/qede.h|  1 -
 drivers/net/ethernet/qlogic/qede/qede_filter.c | 62 --
 drivers/net/ethernet/qlogic/qede/qede_main.c   |  3 --
 3 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede.h 
b/drivers/net/ethernet/qlogic/qede/qede.h
index 9b4f08b..694c09b 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -197,7 +197,6 @@ struct qede_dev {
 #define QEDE_TSS_COUNT(edev)   ((edev)->num_queues - (edev)->fp_num_rx)
 
struct qed_int_info int_info;
-   unsigned char   primary_mac[ETH_ALEN];
 
/* Smaller private varaiant of the RTNL lock */
struct mutexqede_lock;
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c 
b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 333876c..13955a3 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -495,12 +495,16 @@ void qede_force_mac(void *dev, u8 *mac, bool forced)
 {
struct qede_dev *edev = dev;
 
+   __qede_lock(edev);
+
/* MAC hints take effect only if we haven't set one already */
-   if (is_valid_ether_addr(edev->ndev->dev_addr) && !forced)
+   if (is_valid_ether_addr(edev->ndev->dev_addr) && !forced) {
+   __qede_unlock(edev);
return;
+   }
 
ether_addr_copy(edev->ndev->dev_addr, mac);
-   ether_addr_copy(edev->primary_mac, mac);
+   __qede_unlock(edev);
 }
 
 void qede_fill_rss_params(struct qede_dev *edev,
@@ -1061,41 +1065,51 @@ int qede_set_mac_addr(struct net_device *ndev, void *p)
 {
struct qede_dev *edev = netdev_priv(ndev);
struct sockaddr *addr = p;
-   int rc;
-
-   ASSERT_RTNL(); /* @@@TBD To be removed */
+   int rc = 0;
 
-   DP_INFO(edev, "Set_mac_addr called\n");
+   /* Make sure the state doesn't transition while changing the MAC.
+* Also, all flows accessing the dev_addr field are doing that under
+* this lock.
+*/
+   __qede_lock(edev);
 
if (!is_valid_ether_addr(addr->sa_data)) {
DP_NOTICE(edev, "The MAC address is not valid\n");
-   return -EFAULT;
+   rc = -EFAULT;
+   goto out;
}
 
if (!edev->ops->check_mac(edev->cdev, addr->sa_data)) {
-   DP_NOTICE(edev, "qed prevents setting MAC\n");
-   return -EINVAL;
+   DP_NOTICE(edev, "qed prevents setting MAC %pM\n",
+ addr->sa_data);
+   rc = -EINVAL;
+   goto out;
+   }
+
+   if (edev->state == QEDE_STATE_OPEN) {
+   /* Remove the previous primary mac */
+   rc = qede_set_ucast_rx_mac(edev, QED_FILTER_XCAST_TYPE_DEL,
+  ndev->dev_addr);
+   if (rc)
+   goto out;
}
 
ether_addr_copy(ndev->dev_addr, addr->sa_data);
+   DP_INFO(edev, "Setting device MAC to %pM\n", addr->sa_data);
 
-   if (!netif_running(ndev))  {
-   DP_NOTICE(edev, "The device is currently down\n");
-   return 0;
+   if (edev->state != QEDE_STATE_OPEN) {
+   DP_VERBOSE(edev, NETIF_MSG_IFDOWN,
+  "The device is currently down\n");
+   goto out;
}
 
-   /* Remove the previous primary mac */
-   rc = qede_set_ucast_rx_mac(edev, QED_FILTER_XCAST_TYPE_DEL,
-  edev->primary_mac);
-   if (rc)
-   return rc;
-
-   edev->ops->common->update_mac(edev->cdev, addr->sa_data);
+   edev->ops->common->update_mac(edev->cdev, ndev->dev_addr);
 
-   /* Add MAC filter according to the new unicast HW MAC address */
-   ether_addr_copy(edev->primary_mac, ndev->dev_addr);
-   return qede_set_ucast_rx_mac(edev, QED_FILTER_XCAST_TYPE_ADD,
- edev->primary_mac);
+   rc = qede_set_ucast_rx_mac(edev, QED_FILTER_XCAST_TYPE_ADD,
+  ndev->dev_addr);
+out:
+   __qede_unlock(edev);
+   return rc;
 }
 
 static int
@@ -1200,7 +1214,7 @@ void qede_config_rx_mode(struct net_device *ndev)
 * (configrue / leave the primar

[PATCH net-next 02/10] qede: Honor user request for Tx buffers

2017-05-21 Thread Yuval Mintz
From: Sudarsana Reddy Kalluru 

Driver always allocates the maximal number of tx-buffers irrespective of
actual Tx ring config.

Signed-off-by: Sudarsana Reddy Kalluru 
Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c |  6 +++---
 drivers/net/ethernet/qlogic/qede/qede_fp.c  | 18 +-
 drivers/net/ethernet/qlogic/qede/qede_main.c|  6 +++---
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c 
b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 172b292..47ec4f3 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -1297,7 +1297,7 @@ static int qede_selftest_transmit_traffic(struct qede_dev 
*edev,
}
 
/* Fill the entry in the SW ring and the BDs in the FW ring */
-   idx = txq->sw_tx_prod & NUM_TX_BDS_MAX;
+   idx = txq->sw_tx_prod;
txq->sw_tx_ring.skbs[idx].skb = skb;
first_bd = qed_chain_produce(&txq->tx_pbl);
memset(first_bd, 0, sizeof(*first_bd));
@@ -1317,7 +1317,7 @@ static int qede_selftest_transmit_traffic(struct qede_dev 
*edev,
 
/* update the first BD with the actual num BDs */
first_bd->data.nbds = 1;
-   txq->sw_tx_prod++;
+   txq->sw_tx_prod = (txq->sw_tx_prod + 1) % txq->num_tx_buffers;
/* 'next page' entries are counted in the producer value */
val = cpu_to_le16(qed_chain_get_prod_idx(&txq->tx_pbl));
txq->tx_db.data.bd_prod = val;
@@ -1351,7 +1351,7 @@ static int qede_selftest_transmit_traffic(struct qede_dev 
*edev,
first_bd = (struct eth_tx_1st_bd *)qed_chain_consume(&txq->tx_pbl);
dma_unmap_single(&edev->pdev->dev, BD_UNMAP_ADDR(first_bd),
 BD_UNMAP_LEN(first_bd), DMA_TO_DEVICE);
-   txq->sw_tx_cons++;
+   txq->sw_tx_cons = (txq->sw_tx_cons + 1) % txq->num_tx_buffers;
txq->sw_tx_ring.skbs[idx].skb = NULL;
 
return 0;
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c 
b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 7b6f41d..38c8265 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -99,7 +99,7 @@ int qede_alloc_rx_buffer(struct qede_rx_queue *rxq, bool 
allow_lazy)
 /* Unmap the data and free skb */
 int qede_free_tx_pkt(struct qede_dev *edev, struct qede_tx_queue *txq, int 
*len)
 {
-   u16 idx = txq->sw_tx_cons & NUM_TX_BDS_MAX;
+   u16 idx = txq->sw_tx_cons;
struct sk_buff *skb = txq->sw_tx_ring.skbs[idx].skb;
struct eth_tx_1st_bd *first_bd;
struct eth_tx_bd *tx_data_bd;
@@ -156,7 +156,7 @@ static void qede_free_failed_tx_pkt(struct qede_tx_queue 
*txq,
struct eth_tx_1st_bd *first_bd,
int nbd, bool data_split)
 {
-   u16 idx = txq->sw_tx_prod & NUM_TX_BDS_MAX;
+   u16 idx = txq->sw_tx_prod;
struct sk_buff *skb = txq->sw_tx_ring.skbs[idx].skb;
struct eth_tx_bd *tx_data_bd;
int i, split_bd_len = 0;
@@ -333,8 +333,8 @@ static int qede_xdp_xmit(struct qede_dev *edev, struct 
qede_fastpath *fp,
 struct sw_rx_data *metadata, u16 padding, u16 length)
 {
struct qede_tx_queue *txq = fp->xdp_tx;
-   u16 idx = txq->sw_tx_prod & NUM_TX_BDS_MAX;
struct eth_tx_1st_bd *first_bd;
+   u16 idx = txq->sw_tx_prod;
 
if (!qed_chain_get_elem_left(&txq->tx_pbl)) {
txq->stopped_cnt++;
@@ -363,7 +363,7 @@ static int qede_xdp_xmit(struct qede_dev *edev, struct 
qede_fastpath *fp,
 
txq->sw_tx_ring.xdp[idx].page = metadata->data;
txq->sw_tx_ring.xdp[idx].mapping = metadata->mapping;
-   txq->sw_tx_prod++;
+   txq->sw_tx_prod = (txq->sw_tx_prod + 1) % txq->num_tx_buffers;
 
/* Mark the fastpath for future XDP doorbell */
fp->xdp_xmit = 1;
@@ -393,14 +393,14 @@ static void qede_xdp_tx_int(struct qede_dev *edev, struct 
qede_tx_queue *txq)
 
while (hw_bd_cons != qed_chain_get_cons_idx(&txq->tx_pbl)) {
qed_chain_consume(&txq->tx_pbl);
-   idx = txq->sw_tx_cons & NUM_TX_BDS_MAX;
+   idx = txq->sw_tx_cons;
 
dma_unmap_page(&edev->pdev->dev,
   txq->sw_tx_ring.xdp[idx].mapping,
   PAGE_SIZE, DMA_BIDIRECTIONAL);
__free_page(txq->sw_tx_ring.xdp[idx].page);
 
-   txq->sw_tx_cons++;
+   txq->sw_tx_cons = (txq->sw_tx_cons + 1) % txq->num_tx_buffers;
txq->xmit_pkts++;
}
 }
@@ -430,7 +430,7 @@ static int qede_tx_int(struct qede_dev *edev, struct 
qede_tx_queue *txq)
 
bytes_compl += len;
pkts_compl++;
-   txq->sw_tx_cons++;
+   txq->sw_tx_cons = (txq->sw_tx_cons + 1) % txq->num_tx_buffers;
txq->xmit_pkts++;
}
 
@@ -1455,7 +

[PATCH net-next 01/10] qede: Allow WoL to activate by default

2017-05-21 Thread Yuval Mintz
When management firmware declares that the device is WoL-capable,
the default driver behavior would be to allow the management firmware
to take the decision of whether it's actually needed or not.

Problem is ethtool interface doesn't have a 'default' kind
of option, and user would see the interface WoL as disabled,
which doesn't accurately reflect the actual configuration.
More-so, if the user actually wants to explicitly disable WoL he'd have
to first enable it [otherwise ethtool would block the command].

Instead of allowing management to make the decision, enable WoL by
default on all devices capable of it.

Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qede/qede_main.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c 
b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 38b77bb..4a46052 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -618,6 +618,12 @@ static struct qede_dev *qede_alloc_etherdev(struct qed_dev 
*cdev,
memset(&edev->stats, 0, sizeof(edev->stats));
memcpy(&edev->dev_info, info, sizeof(*info));
 
+   /* As ethtool doesn't have the ability to show WoL behavior as
+* 'default', if device supports it declare it's enabled.
+*/
+   if (edev->dev_info.common.wol_support)
+   edev->wol_enabled = true;
+
INIT_LIST_HEAD(&edev->vlan_list);
 
return edev;
-- 
1.9.3



[PATCH net-next 00/10] qed/qede updates

2017-05-21 Thread Yuval Mintz
This series contains some general minor fixes and enhancements:

 - #1, #2 and #9 correct small missing ethtool functionality.
 - #3, #6  and #8 correct minor issues in driver, but those are either
   print-related or unexposed in existing code.
 - #4 adds proper support to TLB mode bonding.
 - #10 is meant to improve performance on varying cache-line sizes.

Dave,

Please consider applying this to `net-next'.

Thanks,
Yuval

Sudarsana Reddy Kalluru (3):
  qede: Support 1G advertisment
  qede: Honor user request for Tx buffers
  qede: Add missing Status-block free

Tomer Tayar (2):
  qed: Revise alloc/setup/free flow
  qed: Fix setting of Management bitfields

Yuval Mintz (5):
  qede: Allow WoL to activate by default
  qede: Don't use an internal MAC field
  qed: Correct print in iscsi error-flow
  qede: qedr closure after setting state
  qed: Cache alignemnt padding to match host

 drivers/net/ethernet/qlogic/qed/qed.h   |  2 +-
 drivers/net/ethernet/qlogic/qed/qed_dcbx.c  |  1 +
 drivers/net/ethernet/qlogic/qed/qed_dev.c   | 94 -
 drivers/net/ethernet/qlogic/qed/qed_fcoe.c  | 23 +++---
 drivers/net/ethernet/qlogic/qed/qed_fcoe.h  | 22 ++
 drivers/net/ethernet/qlogic/qed/qed_init_ops.c  |  4 ++
 drivers/net/ethernet/qlogic/qed/qed_int.c   |  3 +
 drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 24 ---
 drivers/net/ethernet/qlogic/qed/qed_iscsi.h | 23 +++---
 drivers/net/ethernet/qlogic/qed/qed_ll2.c   | 21 +++---
 drivers/net/ethernet/qlogic/qed/qed_ll2.h   | 13 ++--
 drivers/net/ethernet/qlogic/qed/qed_mcp.c   |  1 +
 drivers/net/ethernet/qlogic/qed/qed_ooo.c   | 30 
 drivers/net/ethernet/qlogic/qed/qed_ooo.h   | 26 +++
 drivers/net/ethernet/qlogic/qed/qed_reg_addr.h  |  1 +
 drivers/net/ethernet/qlogic/qed/qed_sp.h| 32 +++--
 drivers/net/ethernet/qlogic/qed/qed_spq.c   | 54 --
 drivers/net/ethernet/qlogic/qede/qede.h |  1 -
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 14 +++-
 drivers/net/ethernet/qlogic/qede/qede_filter.c  | 62 +---
 drivers/net/ethernet/qlogic/qede/qede_fp.c  | 18 ++---
 drivers/net/ethernet/qlogic/qede/qede_main.c| 29 +---
 22 files changed, 269 insertions(+), 229 deletions(-)

-- 
1.9.3



Re: [PATCH 4.4-only] openvswitch: clear sender cpu before forwarding packets

2017-05-21 Thread kbuild test robot
Hi Anoob,

[auto build test ERROR on v4.9-rc8]
[also build test ERROR on next-20170519]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Anoob-Soman/openvswitch-clear-sender-cpu-before-forwarding-packets/20170519-111009
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net/openvswitch/vport.c: In function 'ovs_vport_send':
>> net/openvswitch/vport.c:513:2: error: implicit declaration of function 
>> 'skb_sender_cpu_clear' [-Werror=implicit-function-declaration]
 skb_sender_cpu_clear(skb);
 ^~~~
   cc1: some warnings being treated as errors

vim +/skb_sender_cpu_clear +513 net/openvswitch/vport.c

   507   packet_length(skb), mtu);
   508  vport->dev->stats.tx_errors++;
   509  goto drop;
   510  }
   511  
   512  skb->dev = vport->dev;
 > 513  skb_sender_cpu_clear(skb);
   514  vport->ops->send(skb);
   515  return;
   516  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip