Re: [PATCH net-next RFC] Generic XDP

[Andy Gospodarek]

On Sat, Apr 8, 2017 at 11:07 PM, David Miller  wrote:
>
> This provides a generic non-optimized XDP implementation when the
> device driver does not provide an optimized one.
>
> It is arguable that perhaps I should have required something like
> this as part of the initial XDP feature merge.
>
> I believe this is critical for two reasons:
>
> 1) Accessibility.  More people can play with XDP with less
>dependencies.  Yes I know we have XDP support in virtio_net, but
>that just creates another depedency for learning how to use this
>facility.
>
>I wrote this to make life easier for the XDP newbies.
>
> 2) As a model for what the expected semantics are.  If there is a pure
>generic core implementation, it serves as a semantic example for
>driver folks adding XDP support.
>
> This is just a rough draft and is untested.

Overall I like the idea.  It does reduce the complexity and would
allow anyone to kick the tires with XDP more easily. I did some quick
inspection of the offset calculations coming back from the program and
I _think_ that looks correct.  It might also be too late.  :-)

I should be able to run this on Monday and see how the performance
compares to the driver/native XDP case on some bnxt_en-based hardware.

>
> Signed-off-by: David S. Miller 
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index cc07c3b..7cfb355 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1892,6 +1892,7 @@ struct net_device {
> struct lock_class_key   *qdisc_tx_busylock;
> struct lock_class_key   *qdisc_running_key;
> boolproto_down;
> +   struct bpf_prog *xdp_prog;
>  };
>  #define to_net_dev(d) container_of(d, struct net_device, dev)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7efb417..e4c7927 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -95,6 +95,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -4247,6 +4248,80 @@ static int __netif_receive_skb(struct sk_buff *skb)
> return ret;
>  }
>
> +static struct static_key generic_xdp_needed __read_mostly;
> +
> +static int generic_xdp_install(struct net_device *dev, struct netdev_xdp 
> *xdp)
> +{
> +   struct bpf_prog *new = xdp->prog;
> +   int ret = 0;
> +
> +   switch (xdp->command) {
> +   case XDP_SETUP_PROG: {
> +   struct bpf_prog *old;
> +
> +   old = xchg(>xdp_prog, new);
> +   if (old)
> +   bpf_prog_put(old);
> +
> +   if (old && !new)
> +   static_key_slow_dec(_xdp_needed);
> +   else if (new && !old)
> +   static_key_slow_inc(_xdp_needed);
> +   break;
> +   }
> +   case XDP_QUERY_PROG:
> +   xdp->prog_attached = !!dev->xdp_prog;
> +   ret = 0;
> +   break;
> +   default:
> +   ret = -EINVAL;
> +   break;
> +   }
> +
> +   return ret;
> +}
> +
> +static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> +struct bpf_prog *xdp_prog)
> +{
> +   struct xdp_buff xdp;
> +   u32 act = XDP_DROP;
> +   void *orig_data;
> +   int hlen, off;
> +
> +   if (skb_linearize(skb))
> +   goto do_drop;
> +
> +   hlen = skb_headlen(skb);
> +   xdp.data = skb->data;
> +   xdp.data_end = xdp.data + hlen;
> +   xdp.data_hard_start = xdp.data - skb_headroom(skb);
> +   orig_data = xdp.data;
> +
> +   act = bpf_prog_run_xdp(xdp_prog, );
> +
> +   off = xdp.data - orig_data;
> +   if (off)
> +   __skb_push(skb, off);
> +
> +   switch (act) {
> +   case XDP_PASS:
> +   case XDP_TX:
> +   break;
> +
> +   default:
> +   bpf_warn_invalid_xdp_action(act);
> +   case XDP_ABORTED:
> +   trace_xdp_exception(skb->dev, xdp_prog, act);
> +   case XDP_DROP:
> +   do_drop:
> +   kfree_skb(skb);
> +   break;
> +   }
> +
> +   return act;
> +}
> +
>  static int netif_receive_skb_internal(struct sk_buff *skb)
>  {
> int ret;
> @@ -4258,6 +4333,21 @@ static int netif_receive_skb_internal(struct sk_buff 
> *skb)
>
> rcu_read_lock();
>
> +   if (static_key_false(_xdp_needed)) {
> +   struct bpf_prog *xdp_prog = READ_ONCE(skb->dev->xdp_prog);
> +
> +   if (xdp_prog) {
> +   u32 act = netif_receive_generic_xdp(skb, xdp_prog);
> +
> +   if (act != XDP_PASS) {
> +   rcu_read_unlock();
> +   if (act == XDP_TX)
> +   dev_queue_xmit(skb);
> +   return NET_RX_DROP;
> +   }
> +   }
> +   }
> 

[PATCH net-next 7/7] ibmvnic: Remove inflight list

[Nathan Fontenot]

The inflight list used to track memory that is allocated for crq that are
inflight is not needed. The one piece of the inflight list that does need
to be cleaned at module exit is the error buffer list which is already
attached to the adapter struct.

This patch removes the inflight list and moves checking the error buffer
list to ibmvnic_remove.

Signed-off-by: Nathan Fontenot 
---
 drivers/net/ethernet/ibm/ibmvnic.c |   98 +++-
 drivers/net/ethernet/ibm/ibmvnic.h |9 ---
 2 files changed, 19 insertions(+), 88 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 5de8bae..152332b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -546,6 +546,23 @@ static int init_bounce_buffer(struct net_device *netdev)
return 0;
 }
 
+static void release_error_buffers(struct ibmvnic_adapter *adapter)
+{
+   struct device *dev = >vdev->dev;
+   struct ibmvnic_error_buff *error_buff, *tmp;
+   unsigned long flags;
+
+   spin_lock_irqsave(>error_list_lock, flags);
+   list_for_each_entry_safe(error_buff, tmp, >errors, list) {
+   list_del(_buff->list);
+   dma_unmap_single(dev, error_buff->dma, error_buff->len,
+DMA_FROM_DEVICE);
+   kfree(error_buff->buff);
+   kfree(error_buff);
+   }
+   spin_unlock_irqrestore(>error_list_lock, flags);
+}
+
 static int ibmvnic_login(struct net_device *netdev)
 {
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -588,6 +605,7 @@ static void release_resources(struct ibmvnic_adapter 
*adapter)
release_crq_queue(adapter);
 
release_stats_token(adapter);
+   release_error_buffers(adapter);
 }
 
 static int ibmvnic_open(struct net_device *netdev)
@@ -1959,13 +1977,11 @@ static void send_login(struct ibmvnic_adapter *adapter)
 {
struct ibmvnic_login_rsp_buffer *login_rsp_buffer;
struct ibmvnic_login_buffer *login_buffer;
-   struct ibmvnic_inflight_cmd *inflight_cmd;
struct device *dev = >vdev->dev;
dma_addr_t rsp_buffer_token;
dma_addr_t buffer_token;
size_t rsp_buffer_size;
union ibmvnic_crq crq;
-   unsigned long flags;
size_t buffer_size;
__be64 *tx_list_p;
__be64 *rx_list_p;
@@ -2002,11 +2018,7 @@ static void send_login(struct ibmvnic_adapter *adapter)
dev_err(dev, "Couldn't map login rsp buffer\n");
goto buf_rsp_map_failed;
}
-   inflight_cmd = kmalloc(sizeof(*inflight_cmd), GFP_ATOMIC);
-   if (!inflight_cmd) {
-   dev_err(dev, "Couldn't allocate inflight_cmd\n");
-   goto inflight_alloc_failed;
-   }
+
adapter->login_buf = login_buffer;
adapter->login_buf_token = buffer_token;
adapter->login_buf_sz = buffer_size;
@@ -2057,20 +2069,10 @@ static void send_login(struct ibmvnic_adapter *adapter)
crq.login.cmd = LOGIN;
crq.login.ioba = cpu_to_be32(buffer_token);
crq.login.len = cpu_to_be32(buffer_size);
-
-   memcpy(_cmd->crq, , sizeof(crq));
-
-   spin_lock_irqsave(>inflight_lock, flags);
-   list_add_tail(_cmd->list, >inflight);
-   spin_unlock_irqrestore(>inflight_lock, flags);
-
ibmvnic_send_crq(adapter, );
 
return;
 
-inflight_alloc_failed:
-   dma_unmap_single(dev, rsp_buffer_token, rsp_buffer_size,
-DMA_FROM_DEVICE);
 buf_rsp_map_failed:
kfree(login_rsp_buffer);
 buf_rsp_alloc_failed:
@@ -2376,7 +2378,6 @@ static void handle_error_indication(union ibmvnic_crq 
*crq,
struct ibmvnic_adapter *adapter)
 {
int detail_len = be32_to_cpu(crq->error_indication.detail_error_sz);
-   struct ibmvnic_inflight_cmd *inflight_cmd;
struct device *dev = >vdev->dev;
struct ibmvnic_error_buff *error_buff;
union ibmvnic_crq new_crq;
@@ -2408,15 +2409,6 @@ static void handle_error_indication(union ibmvnic_crq 
*crq,
return;
}
 
-   inflight_cmd = kmalloc(sizeof(*inflight_cmd), GFP_ATOMIC);
-   if (!inflight_cmd) {
-   dma_unmap_single(dev, error_buff->dma, detail_len,
-DMA_FROM_DEVICE);
-   kfree(error_buff->buff);
-   kfree(error_buff);
-   return;
-   }
-
error_buff->len = detail_len;
error_buff->error_id = crq->error_indication.error_id;
 
@@ -2430,13 +2422,6 @@ static void handle_error_indication(union ibmvnic_crq 
*crq,
new_crq.request_error_info.ioba = cpu_to_be32(error_buff->dma);
new_crq.request_error_info.len = cpu_to_be32(detail_len);
new_crq.request_error_info.error_id = crq->error_indication.error_id;
-
-   memcpy(_cmd->crq, , sizeof(crq));
-
-   spin_lock_irqsave(>inflight_lock, 

[PATCH net-next 6/7] ibmvnic: Do not disable IRQ after scheduling tasklet

[Nathan Fontenot]

From: Brian King 

Since the primary CRQ is only used for service functions and
not in the performance path, simplify the code a bit and avoid
disabling the IRQ.

Signed-off-by: Brian King 
Signed-off-by: Nathan Fontenot 
---
 drivers/net/ethernet/ibm/ibmvnic.c |   25 ++---
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 4e27518..5de8bae 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3031,12 +3031,8 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 static irqreturn_t ibmvnic_interrupt(int irq, void *instance)
 {
struct ibmvnic_adapter *adapter = instance;
-   unsigned long flags;
 
-   spin_lock_irqsave(>crq.lock, flags);
-   vio_disable_interrupts(adapter->vdev);
tasklet_schedule(>tasklet);
-   spin_unlock_irqrestore(>crq.lock, flags);
return IRQ_HANDLED;
 }
 
@@ -3044,32 +3040,23 @@ static void ibmvnic_tasklet(void *data)
 {
struct ibmvnic_adapter *adapter = data;
struct ibmvnic_crq_queue *queue = >crq;
-   struct vio_dev *vdev = adapter->vdev;
union ibmvnic_crq *crq;
unsigned long flags;
bool done = false;
 
spin_lock_irqsave(>lock, flags);
-   vio_disable_interrupts(vdev);
while (!done) {
/* Pull all the valid messages off the CRQ */
while ((crq = ibmvnic_next_crq(adapter)) != NULL) {
ibmvnic_handle_crq(crq, adapter);
crq->generic.first = 0;
}
-   vio_enable_interrupts(vdev);
-   crq = ibmvnic_next_crq(adapter);
-   if (crq) {
-   vio_disable_interrupts(vdev);
-   ibmvnic_handle_crq(crq, adapter);
-   crq->generic.first = 0;
-   } else {
-   /* remain in tasklet until all
-* capabilities responses are received
-*/
-   if (!adapter->wait_capability)
-   done = true;
-   }
+   
+   /* remain in tasklet until all
+* capabilities responses are received
+*/
+   if (!adapter->wait_capability)
+   done = true;
}
/* if capabilities CRQ's were sent in this tasklet, the following
 * tasklet must wait until all responses are received

[PATCH net-next 3/7] ibmvnic: Fix ibmvnic_change_mac_addr struct format

[Nathan Fontenot]

From: Murilo Fossa Vicentini 

The ibmvnic_change_mac_addr struct alignment was not matching the defined
format in PAPR+, it had the reserved and return code fields swapped. As a
consequence, the CHANGE_MAC_ADDR_RSP commands were being improperly handled
and executed even when the operation wasn't successfully completed by the
system firmware.

Also changing the endianness of the debug message to make it easier to
parse the CRQ content.

Signed-off-by: Murilo Fossa Vicentini 
Signed-off-by: Nathan Fontenot 
---
 drivers/net/ethernet/ibm/ibmvnic.c |5 +++--
 drivers/net/ethernet/ibm/ibmvnic.h |2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index b791361..2e4d787 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2892,11 +2892,12 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
struct ibmvnic_generic_crq *gen_crq = >generic;
struct net_device *netdev = adapter->netdev;
struct device *dev = >vdev->dev;
+   u64 *u64_crq = (u64 *)crq;
long rc;
 
netdev_dbg(netdev, "Handling CRQ: %016lx %016lx\n",
-  ((unsigned long int *)crq)[0],
-  ((unsigned long int *)crq)[1]);
+  (unsigned long int)cpu_to_be64(u64_crq[0]),
+  (unsigned long int)cpu_to_be64(u64_crq[1]));
switch (gen_crq->first) {
case IBMVNIC_CRQ_INIT_RSP:
switch (gen_crq->cmd) {
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h 
b/drivers/net/ethernet/ibm/ibmvnic.h
index 0a8f9a0..e6b0e60 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -518,8 +518,8 @@ struct ibmvnic_change_mac_addr {
u8 first;
u8 cmd;
u8 mac_addr[6];
-   struct ibmvnic_rc rc;
u8 reserved[4];
+   struct ibmvnic_rc rc;
 } __packed __aligned(8);
 
 struct ibmvnic_multicast_ctrl {

[PATCH net-next 5/7] ibmvnic: Fixup atomic API usage

[Nathan Fontenot]

From: Brian King 

Replace a couple of modifications of an atomic followed
by a read of the atomic, which is no longer atomic, to
use atomic_XX_return variants to avoid race conditions.

Signed-off-by: Brian King 
Signed-off-by: Nathan Fontenot 
---
 drivers/net/ethernet/ibm/ibmvnic.c |   10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 5f9aad8..4e27518 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -964,9 +964,8 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct 
net_device *netdev)
goto out;
}
 
-   atomic_inc(_scrq->used);
-
-   if (atomic_read(_scrq->used) >= adapter->req_tx_entries_per_subcrq) {
+   if (atomic_inc_return(_scrq->used) 
+   >= adapter->req_tx_entries_per_subcrq) {
netdev_info(netdev, "Stopping queue %d\n", queue_num);
netif_stop_subqueue(netdev, queue_num);
}
@@ -1501,9 +1500,8 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter 
*adapter,
}
 
if (txbuff->last_frag) {
-   atomic_dec(>used);
-
-   if (atomic_read(>used) <=
+   if (atomic_sub_return(next->tx_comp.num_comps,
+ >used) <=
(adapter->req_tx_entries_per_subcrq / 2) &&
netif_subqueue_stopped(adapter->netdev,
   txbuff->skb)) {

[PATCH net-next 4/7] ibmvnic: Unmap longer term buffer before free

[Nathan Fontenot]

From: Brian King 

Make sure we unregister long term buffers from the adapter
prior to DMA unmapping it and freeing the buffer. Failure
to do so could result in a DMA to a now invalid address.

Signed-off-by: Brian King 
Signed-off-by: Nathan Fontenot 
---
 drivers/net/ethernet/ibm/ibmvnic.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2e4d787..5f9aad8 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -193,9 +193,9 @@ static void free_long_term_buff(struct ibmvnic_adapter 
*adapter,
if (!ltb->buff)
return;
 
-   dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
if (!adapter->failover)
send_request_unmap(adapter, ltb->map_id);
+   dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
 }
 
 static void replenish_rx_pool(struct ibmvnic_adapter *adapter,

[PATCH net-next 0/7] ibmvnic: Assorted driver updates

[Nathan Fontenot]

This is a set of updates to the ibmvnic driver to fix severalbugs in the
ibmvnic driver.

Patch 1/7: Add is_up flag to avoid sending packets when drivern is down
Patch 2/7: Report errors releasing sub-crqs
Patch 3/7: Correct change_mac_addr struct format
Patch 4/7: Unmap long term buffers before free'ing them
Patch 5/7: Fix up atomic API usage
Patch 6/7: Don't disable irq after scheduling tasklet
Patch 7/7: Remove inflight tracker

-Nathan
---

Nathan Fontenot (1):
  ibmvnic: Remove inflight list

Thomas Falcon (2):
  ibmvnic: Add is_up flag to avoid transmits when driver is down
  ibmvnic: Report errors when failing to release sub-crqs

Brian King (3):
  ibmvnic: Unmap longer term buffer before free
  ibmvnic: Fixup atomic API usage
  ibmvnic: Do not disable IRQ after scheduling tasklet

Murilo Fossa Vicentini (1):
  ibmvnic: Fix ibmvnic_change_mac_addr struct format

 drivers/net/ethernet/ibm/ibmvnic.c |  162 +++-
 drivers/net/ethernet/ibm/ibmvnic.h |   12 ---
 2 files changed, 51 insertions(+), 123 deletions(-)

[PATCH net-next 2/7] ibmvnic: Report errors when failing to release sub-crqs

[Nathan Fontenot]

From: Thomas Falcon 

Add reporting of errors when releasing sub-crqs fails.

Signed-off-by: Thomas Falcon 
Signed-off-by: Nathan Fontenot 
---
 drivers/net/ethernet/ibm/ibmvnic.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2722f07..b791361 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1311,6 +1311,12 @@ static void release_sub_crq_queue(struct ibmvnic_adapter 
*adapter,
scrq->crq_num);
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
 
+   if (rc) {
+   netdev_err(adapter->netdev,
+  "Failed to release sub-CRQ %16lx, rc = %ld\n",
+  scrq->crq_num, rc);
+   }
+
dma_unmap_single(dev, scrq->msg_token, 4 * PAGE_SIZE,
 DMA_BIDIRECTIONAL);
free_pages((unsigned long)scrq->msgs, 2);

[PATCH net-next 1/7] ibmvnic: Add is_up flag to avoid transmits when driver is down

[Nathan Fontenot]

From: Thomas Falcon 

There are brief windows when handling events such as failover where we
could attempt to transmit packets between receiving the transport event
notification and handling the reset in the workqueue.

This patch introduces an is_up flag so we can avoid transmit attempts at
these times.

Signed-off-by: Thomas Falcon 
Signed-off-by: Nathan Fontenot 
---
 drivers/net/ethernet/ibm/ibmvnic.c |   16 ++--
 drivers/net/ethernet/ibm/ibmvnic.h |1 +
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7ba43cf..2722f07 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -664,6 +664,7 @@ static int ibmvnic_open(struct net_device *netdev)
 
netif_tx_start_all_queues(netdev);
adapter->is_closed = false;
+   adapter->is_up = true;
 
return 0;
 
@@ -857,18 +858,19 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct 
net_device *netdev)
int index = 0;
int ret = 0;
 
+   if (adapter->migrated || !adapter->is_up) {
+   tx_send_failed++;
+   tx_dropped++;
+   ret = NETDEV_TX_BUSY;
+   goto out;
+   }
+
tx_pool = >tx_pool[queue_num];
tx_scrq = adapter->tx_scrq[queue_num];
txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
handle_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
   be32_to_cpu(adapter->login_rsp_buf->
   off_txsubm_subcrqs));
-   if (adapter->migrated) {
-   tx_send_failed++;
-   tx_dropped++;
-   ret = NETDEV_TX_BUSY;
-   goto out;
-   }
 
index = tx_pool->free_map[tx_pool->consumer_index];
offset = index * adapter->req_mtu;
@@ -2913,11 +2915,13 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
if (gen_crq->cmd == IBMVNIC_PARTITION_MIGRATED) {
dev_info(dev, "Re-enabling adapter\n");
adapter->migrated = true;
+   adapter->is_up = false;
schedule_work(>ibmvnic_xport);
} else if (gen_crq->cmd == IBMVNIC_DEVICE_FAILOVER) {
dev_info(dev, "Backing device failover detected\n");
netif_carrier_off(netdev);
adapter->failover = true;
+   adapter->is_up = false;
} else {
/* The adapter lost the connection */
dev_err(dev, "Virtual Adapter failed (rc=%d)\n",
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h 
b/drivers/net/ethernet/ibm/ibmvnic.h
index b0d0b89..0a8f9a0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -1022,4 +1022,5 @@ struct ibmvnic_adapter {
struct tasklet_struct tasklet;
bool failover;
bool is_closed;
+   bool is_up;
 };

[PATCH net-next RFC] Generic XDP

[David Miller]

This provides a generic non-optimized XDP implementation when the
device driver does not provide an optimized one.

It is arguable that perhaps I should have required something like
this as part of the initial XDP feature merge.

I believe this is critical for two reasons:

1) Accessibility.  More people can play with XDP with less
   dependencies.  Yes I know we have XDP support in virtio_net, but
   that just creates another depedency for learning how to use this
   facility.

   I wrote this to make life easier for the XDP newbies.

2) As a model for what the expected semantics are.  If there is a pure
   generic core implementation, it serves as a semantic example for
   driver folks adding XDP support.

This is just a rough draft and is untested.

Signed-off-by: David S. Miller 

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cc07c3b..7cfb355 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1892,6 +1892,7 @@ struct net_device {
struct lock_class_key   *qdisc_tx_busylock;
struct lock_class_key   *qdisc_running_key;
boolproto_down;
+   struct bpf_prog *xdp_prog;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 7efb417..e4c7927 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -95,6 +95,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -4247,6 +4248,80 @@ static int __netif_receive_skb(struct sk_buff *skb)
return ret;
 }
 
+static struct static_key generic_xdp_needed __read_mostly;
+
+static int generic_xdp_install(struct net_device *dev, struct netdev_xdp *xdp)
+{
+   struct bpf_prog *new = xdp->prog;
+   int ret = 0;
+
+   switch (xdp->command) {
+   case XDP_SETUP_PROG: {
+   struct bpf_prog *old;
+
+   old = xchg(>xdp_prog, new);
+   if (old)
+   bpf_prog_put(old);
+
+   if (old && !new)
+   static_key_slow_dec(_xdp_needed);
+   else if (new && !old)
+   static_key_slow_inc(_xdp_needed);
+   break;
+   }
+   case XDP_QUERY_PROG:
+   xdp->prog_attached = !!dev->xdp_prog;
+   ret = 0;
+   break;
+   default:
+   ret = -EINVAL;
+   break;
+   }
+
+   return ret;
+}
+
+static u32 netif_receive_generic_xdp(struct sk_buff *skb,
+struct bpf_prog *xdp_prog)
+{
+   struct xdp_buff xdp;
+   u32 act = XDP_DROP;
+   void *orig_data;
+   int hlen, off;
+
+   if (skb_linearize(skb))
+   goto do_drop;
+
+   hlen = skb_headlen(skb);
+   xdp.data = skb->data;
+   xdp.data_end = xdp.data + hlen;
+   xdp.data_hard_start = xdp.data - skb_headroom(skb);
+   orig_data = xdp.data;
+
+   act = bpf_prog_run_xdp(xdp_prog, );
+
+   off = xdp.data - orig_data;
+   if (off)
+   __skb_push(skb, off);
+
+   switch (act) {
+   case XDP_PASS:
+   case XDP_TX:
+   break;
+
+   default:
+   bpf_warn_invalid_xdp_action(act);
+   case XDP_ABORTED:
+   trace_xdp_exception(skb->dev, xdp_prog, act);
+   case XDP_DROP:
+   do_drop:
+   kfree_skb(skb);
+   break;
+   }
+
+   return act;
+}
+
 static int netif_receive_skb_internal(struct sk_buff *skb)
 {
int ret;
@@ -4258,6 +4333,21 @@ static int netif_receive_skb_internal(struct sk_buff 
*skb)
 
rcu_read_lock();
 
+   if (static_key_false(_xdp_needed)) {
+   struct bpf_prog *xdp_prog = READ_ONCE(skb->dev->xdp_prog);
+
+   if (xdp_prog) {
+   u32 act = netif_receive_generic_xdp(skb, xdp_prog);
+
+   if (act != XDP_PASS) {
+   rcu_read_unlock();
+   if (act == XDP_TX)
+   dev_queue_xmit(skb);
+   return NET_RX_DROP;
+   }
+   }
+   }
+
 #ifdef CONFIG_RPS
if (static_key_false(_needed)) {
struct rps_dev_flow voidflow, *rflow = 
@@ -6718,6 +6808,7 @@ EXPORT_SYMBOL(dev_change_proto_down);
  */
 int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags)
 {
+   int (*xdp_op)(struct net_device *dev, struct netdev_xdp *xdp);
const struct net_device_ops *ops = dev->netdev_ops;
struct bpf_prog *prog = NULL;
struct netdev_xdp xdp;
@@ -6725,14 +6816,16 @@ int dev_change_xdp_fd(struct net_device *dev, int fd, 
u32 flags)
 
ASSERT_RTNL();
 
-   if (!ops->ndo_xdp)
-   return -EOPNOTSUPP;
+   xdp_op = ops->ndo_xdp;
+   if (!xdp_op)
+   xdp_op = generic_xdp_install;
+
if (fd >= 0) {
   

Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events

[David Ahern]

On 4/8/17 9:36 PM, Vlad Yasevich wrote:
> On 04/07/2017 05:25 PM, David Ahern wrote:
>> Changing MTU on a link currently causes 3 messages to be sent to userspace:
>>
>> [LINK]11: dummy1:  mtu 1490 qdisc noqueue state 
>> UNKNOWN group default event PRE_CHANGE_MTU
>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>
>> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
>> UNKNOWN group default event CHANGE_MTU
>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>
>> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
>> UNKNOWN group default
>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>
>> Remove the PRE_CHANGE_MTU and CHANGE_MTU messages.
> Actually, I have plans for the CHANGE_MTU event.  The last message is not an 
> event. If you
> remove the event, it is much harder to track mtu changes.

it is a set of redundant messages. Roopa suggested using a bitmask for
the events changed by do_setlink which gets added to the last message
seen above. In doing so you only generate 1 message. The above causes
libnl (for example):

- message 1: delete dummy1 from its cache and re-create it
- message 2: delete dummy1 from its cache and re-create it
- message 3: delete dummy1 from its cache and re-create it

All you need is a single message at the end with an attribute that says
"MTU changed"

Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor

[David Ahern]

On 4/8/17 10:33 PM, David Miller wrote:
> From: David Ahern 
> Date: Sat, 8 Apr 2017 18:24:06 -0400
> 
>> per comments on the email thread about reducing notifications, the
>> kernel patch for this should be reverted (and hence this iproute2 patch
>> is not needed) in favor of using a bitmask. Right now there are too many
>> redundant notifications to userspace.
> 
> I must have missed something in all the discussion, which patch needs
> to be reverted from my tree exactly?
> 

Here's the thread:
https://www.spinics.net/lists/netdev/msg429154.html

The comment is that def12888c161 is adding a uapi that leads to way too
many notifications (e.g., on a setlink).

It would be more efficient (read less notifications) to have do_setlink
emit a single message with the IFLA_EVENT (or something else
appropriately named) that indicates what attributes changed. Right now,
a change MTU leads to 3 notifications causing unnecessary churn in
userspace to track what the state of the link is.

Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor

[David Miller]

From: David Ahern 
Date: Sat, 8 Apr 2017 18:24:06 -0400

> per comments on the email thread about reducing notifications, the
> kernel patch for this should be reverted (and hence this iproute2 patch
> is not needed) in favor of using a bitmask. Right now there are too many
> redundant notifications to userspace.

I must have missed something in all the discussion, which patch needs
to be reverted from my tree exactly?

Re: [PATCH net-next 7/8] rtnetlink: Do not generate notifications for NOTIFY_PEERS event

[Vlad Yasevich]

On 04/07/2017 05:25 PM, David Ahern wrote:
> NOTIFY_PEERS is an internal event; do not generate userspace
> notifications.

We actually need this event to support macvtap over bonding
as well as to resolve some issues with VMs using a bonded uplink
on the host.

-vlad

> 
> Signed-off-by: David Ahern 
> ---
>  include/uapi/linux/if_link.h | 1 -
>  net/core/rtnetlink.c | 4 
>  2 files changed, 5 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 4fa3bf3eb21d..8f23e9dde667 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -906,7 +906,6 @@ enum {
>   IFLA_EVENT_CHANGE_NAME,
>   IFLA_EVENT_FEAT_CHANGE,
>   IFLA_EVENT_BONDING_FAILOVER,
> - IFLA_EVENT_NOTIFY_PEERS,
>   IFLA_EVENT_CHANGE_UPPER,
>   IFLA_EVENT_RESEND_IGMP,
>   IFLA_EVENT_CHANGE_LOWER_STATE,
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index e2b0ec5174e7..d2587aa339c4 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1294,9 +1294,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, 
> unsigned long event)
>   case NETDEV_BONDING_FAILOVER:
>   rtnl_event = IFLA_EVENT_BONDING_FAILOVER;
>   break;
> - case NETDEV_NOTIFY_PEERS:
> - rtnl_event = IFLA_EVENT_NOTIFY_PEERS;
> - break;
>   case NETDEV_CHANGEUPPER:
>   rtnl_event = IFLA_EVENT_CHANGE_UPPER;
>   break;
> @@ -4173,7 +4170,6 @@ static int rtnetlink_event(struct notifier_block *this, 
> unsigned long event, voi
>   case NETDEV_CHANGENAME:
>   case NETDEV_FEAT_CHANGE:
>   case NETDEV_BONDING_FAILOVER:
> - case NETDEV_NOTIFY_PEERS:
>   case NETDEV_CHANGEUPPER:
>   case NETDEV_RESEND_IGMP:
>   case NETDEV_CHANGELOWERSTATE:
> 

Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events

[Vlad Yasevich]

On 04/07/2017 05:25 PM, David Ahern wrote:
> Changing MTU on a link currently causes 3 messages to be sent to userspace:
> 
> [LINK]11: dummy1:  mtu 1490 qdisc noqueue state 
> UNKNOWN group default event PRE_CHANGE_MTU
> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
> 
> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
> UNKNOWN group default event CHANGE_MTU
> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
> 
> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
> UNKNOWN group default
> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
> 
> Remove the PRE_CHANGE_MTU and CHANGE_MTU messages.

Actually, I have plans for the CHANGE_MTU event.  The last message is not an 
event. If you
remove the event, it is much harder to track mtu changes.

-vlad
> 
> Signed-off-by: David Ahern 
> ---
>  include/uapi/linux/if_link.h | 2 --
>  net/core/rtnetlink.c | 8 ---
>  2 files changed, 10 deletions(-)
> 

> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 97f6d302f627..e8b7e9342cc0 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -903,7 +903,6 @@ enum {
>  enum {
>   IFLA_EVENT_UNSPEC,
>   IFLA_EVENT_REBOOT,
> - IFLA_EVENT_CHANGE_MTU,
>   IFLA_EVENT_CHANGE_ADDR,
>   IFLA_EVENT_CHANGE_NAME,
>   IFLA_EVENT_FEAT_CHANGE,
> @@ -912,7 +911,6 @@ enum {
>   IFLA_EVENT_NOTIFY_PEERS,
>   IFLA_EVENT_CHANGE_UPPER,
>   IFLA_EVENT_RESEND_IGMP,
> - IFLA_EVENT_PRE_CHANGE_MTU,
>   IFLA_EVENT_CHANGE_INFO_DATA,
>   IFLA_EVENT_PRE_CHANGE_UPPER,
>   IFLA_EVENT_CHANGE_LOWER_STATE,
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index b2bd4c9ee860..72d365ae14b3 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1285,9 +1285,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, 
> unsigned long event)
>   case NETDEV_REBOOT:
>   rtnl_event = IFLA_EVENT_REBOOT;
>   break;
> - case NETDEV_CHANGEMTU:
> - rtnl_event = IFLA_EVENT_CHANGE_MTU;
> - break;
>   case NETDEV_CHANGEADDR:
>   rtnl_event = IFLA_EVENT_CHANGE_ADDR;
>   break;
> @@ -1312,9 +1309,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, 
> unsigned long event)
>   case NETDEV_RESEND_IGMP:
>   rtnl_event = IFLA_EVENT_RESEND_IGMP;
>   break;
> - case NETDEV_PRECHANGEMTU:
> - rtnl_event = IFLA_EVENT_PRE_CHANGE_MTU;
> - break;
>   case NETDEV_CHANGEINFODATA:
>   rtnl_event = IFLA_EVENT_CHANGE_INFO_DATA;
>   break;
> @@ -4191,7 +4185,6 @@ static int rtnetlink_event(struct notifier_block *this, 
> unsigned long event, voi
>  
>   switch (event) {
>   case NETDEV_REBOOT:
> - case NETDEV_CHANGEMTU:
>   case NETDEV_CHANGEADDR:
>   case NETDEV_CHANGENAME:
>   case NETDEV_FEAT_CHANGE:
> @@ -4200,7 +4193,6 @@ static int rtnetlink_event(struct notifier_block *this, 
> unsigned long event, voi
>   case NETDEV_NOTIFY_PEERS:
>   case NETDEV_CHANGEUPPER:
>   case NETDEV_RESEND_IGMP:
> - case NETDEV_PRECHANGEMTU:
>   case NETDEV_CHANGEINFODATA:
>   case NETDEV_PRECHANGEUPPER:
>   case NETDEV_CHANGELOWERSTATE:
> 

[PATCH net-next,3/3] hv_netvsc: Exclude non-TCP port numbers from vRSS hashing

[Haiyang Zhang]

From: Haiyang Zhang 

Azure hosts are not supporting non-TCP port numbers in vRSS hashing for
now. For example, UDP packet loss rate will be high if port numbers are
also included in vRSS hash.

So, we created this patch to use only IP numbers for hashing in non-TCP
traffic.

Signed-off-by: Haiyang Zhang 
Reviewed-by: Stephen Hemminger 
---
 drivers/net/hyperv/netvsc_drv.c |   38 ++
 1 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index fad864f..d65ab05 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -191,6 +191,39 @@ static int netvsc_close(struct net_device *net)
return ppi;
 }
 
+/* Azure hosts don't support non-TCP port numbers in hashing yet. We compute
+ * hash for non-TCP traffic with only IP numbers.
+ */
+static inline u32 netvsc_get_hash(struct sk_buff *skb, struct sock *sk)
+{
+   struct flow_keys flow;
+   u32 hash;
+   static u32 hashrnd __read_mostly;
+
+   net_get_random_once(, sizeof(hashrnd));
+
+   if (!skb_flow_dissect_flow_keys(skb, , 0))
+   return 0;
+
+   if (flow.basic.ip_proto == IPPROTO_TCP) {
+   if (sk)
+   skb_set_hash_from_sk(skb, sk);
+
+   return skb_get_hash(skb);
+   } else {
+   if (flow.basic.n_proto == htons(ETH_P_IP))
+   hash = jhash2((u32 *), 2, hashrnd);
+   else if (flow.basic.n_proto == htons(ETH_P_IPV6))
+   hash = jhash2((u32 *), 8, hashrnd);
+   else
+   hash = 0;
+   }
+
+   skb_set_hash(skb, hash, PKT_HASH_TYPE_L3);
+
+   return hash;
+}
+
 static inline int netvsc_get_tx_queue(struct net_device *ndev,
  struct sk_buff *skb, int old_idx)
 {
@@ -198,10 +231,7 @@ static inline int netvsc_get_tx_queue(struct net_device 
*ndev,
struct sock *sk = skb->sk;
int q_idx;
 
-   if (sk)
-   skb_set_hash_from_sk(skb, sk);
-
-   q_idx = ndc->tx_send_table[skb_get_hash(skb) &
+   q_idx = ndc->tx_send_table[netvsc_get_hash(skb, sk) &
   (VRSS_SEND_TAB_SIZE - 1)];
 
/* If queue index changed record the new value */
-- 
1.7.1

[PATCH net-next,1/3] hv_netvsc: Use per socket hash when available

[Haiyang Zhang]

From: Haiyang Zhang 

The per socket hash is set when a socket is connected. Use it, when
available, to save CPU cycles on repeatedly computing hash on the same
connection.

Signed-off-by: Haiyang Zhang 
Reviewed-by: Stephen Hemminger 
---
 drivers/net/hyperv/netvsc_drv.c |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f24c289..0a129cb 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -211,9 +211,14 @@ static u16 netvsc_select_queue(struct net_device *ndev, 
struct sk_buff *skb,
int q_idx = sk_tx_queue_get(sk);
 
if (q_idx < 0 || skb->ooo_okay || q_idx >= num_tx_queues) {
-   u16 hash = __skb_tx_hash(ndev, skb, VRSS_SEND_TAB_SIZE);
+   u16 hash;
int new_idx;
 
+   if (sk)
+   skb_set_hash_from_sk(skb, sk);
+
+   hash = __skb_tx_hash(ndev, skb, VRSS_SEND_TAB_SIZE);
+
new_idx = net_device_ctx->tx_send_table[hash] % num_tx_queues;
 
if (q_idx != new_idx && sk &&
-- 
1.7.1

[PATCH net-next,2/3] hv_netvsc: Fix the queue index computation in forwarding case

[Haiyang Zhang]

From: Haiyang Zhang 

If the outgoing skb has a RX queue mapping available, we use the queue
number directly, other than put it through Send Indirection Table.

Signed-off-by: Haiyang Zhang 
Reviewed-by: Stephen Hemminger 
---
 drivers/net/hyperv/hyperv_net.h |2 +-
 drivers/net/hyperv/netvsc_drv.c |   54 --
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 4747ad4..768b3ae 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -633,7 +633,7 @@ struct nvsp_message {
 
 #define NETVSC_PACKET_SIZE  4096
 
-#define VRSS_SEND_TAB_SIZE 16
+#define VRSS_SEND_TAB_SIZE 16  /* must be power of 2 */
 #define VRSS_CHANNEL_MAX 64
 #define VRSS_CHANNEL_DEFAULT 8
 
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 0a129cb..fad864f 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -191,6 +191,27 @@ static int netvsc_close(struct net_device *net)
return ppi;
 }
 
+static inline int netvsc_get_tx_queue(struct net_device *ndev,
+ struct sk_buff *skb, int old_idx)
+{
+   const struct net_device_context *ndc = netdev_priv(ndev);
+   struct sock *sk = skb->sk;
+   int q_idx;
+
+   if (sk)
+   skb_set_hash_from_sk(skb, sk);
+
+   q_idx = ndc->tx_send_table[skb_get_hash(skb) &
+  (VRSS_SEND_TAB_SIZE - 1)];
+
+   /* If queue index changed record the new value */
+   if (q_idx != old_idx &&
+   sk && sk_fullsock(sk) && rcu_access_pointer(sk->sk_dst_cache))
+   sk_tx_queue_set(sk, q_idx);
+
+   return q_idx;
+}
+
 /*
  * Select queue for transmit.
  *
@@ -205,29 +226,22 @@ static int netvsc_close(struct net_device *net)
 static u16 netvsc_select_queue(struct net_device *ndev, struct sk_buff *skb,
void *accel_priv, select_queue_fallback_t fallback)
 {
-   struct net_device_context *net_device_ctx = netdev_priv(ndev);
unsigned int num_tx_queues = ndev->real_num_tx_queues;
-   struct sock *sk = skb->sk;
-   int q_idx = sk_tx_queue_get(sk);
-
-   if (q_idx < 0 || skb->ooo_okay || q_idx >= num_tx_queues) {
-   u16 hash;
-   int new_idx;
-
-   if (sk)
-   skb_set_hash_from_sk(skb, sk);
-
-   hash = __skb_tx_hash(ndev, skb, VRSS_SEND_TAB_SIZE);
+   int q_idx = sk_tx_queue_get(skb->sk);
 
-   new_idx = net_device_ctx->tx_send_table[hash] % num_tx_queues;
-
-   if (q_idx != new_idx && sk &&
-   sk_fullsock(sk) && rcu_access_pointer(sk->sk_dst_cache))
-   sk_tx_queue_set(sk, new_idx);
-
-   q_idx = new_idx;
+   if (q_idx < 0 || skb->ooo_okay) {
+   /* If forwarding a packet, we use the recorded queue when
+* available for better cache locality.
+*/
+   if (skb_rx_queue_recorded(skb))
+   q_idx = skb_get_rx_queue(skb);
+   else
+   q_idx = netvsc_get_tx_queue(ndev, skb, q_idx);
}
 
+   while (unlikely(q_idx >= num_tx_queues))
+   q_idx -= num_tx_queues;
+
return q_idx;
 }
 
-- 
1.7.1

Performance regression on loopback devices when qdiscs are attached in 4.4

[Joseph Lynch]

Hello,

I apologize if this is the wrong list to report this bug to, I did not
find a more specific listing in the maintainers file. I think this is
a kernel issue and not an issue with my distro, but if you disagree I
can re-direct this report as appropriate.

I am upgrading some Linux 4.2 servers to Linux 4.4 (Ubuntu Xenial),
and during testing I'm observing TCP segment re-transmits very
occasionally on the loopback device, leading to 200ms latency spikes.
I don't observe the issues on non loopback devices, and I believe that
I've narrowed it down to an issue with qdiscs on loopback.

It seems that when a queuing discipline other than noqueue is attached
to a loopback device in 4.4+ kernels, packets will (very occasionally)
get dropped completely leading to a re-transmit. I'm not sure how this
can happen, and I've been trying to figure out what's going on, but if
anyone has any pointers or suggestions I'd very much appreciate that.

I've attached the script I'm using to reproduce the bug and an example
ab run that I believe shows the bug. In particular, the max timings of
200ms in the ab output and seeing TCP segment re-transmits (and
sometimes RSTs) in the tcpdump output is indicating the issue to me. I
have tested on 3.13, 4.2 and 4.4 kernels and only 4.4 is showing the
issue. Furthermore non loopback interfaces don't appear to have the
bug. So I ran git diff v4.2..v4.4 drivers/net/loopback.c, and the only
commit that seems to touch loopback.c is e65db2b7. I'm attempting to
revert the change and re-compile to see if that commit triggers the
bug, but I don't understand why that change would be breaking things
in this way so that's just a guess.

I'm continuing to try to debug this, but I figured it would be a good
idea to report it here in case someone with more familiarity may know
what's going on. Please let me know if there is any additional
information I can provide or tests I can run.

Thank you,
-Joey Lynch
This is ApacheBench, Version 2.3 <$Revision: 1528965 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking 127.0.0.1 (be patient)
Completed 1 requests
Completed 2 requests
Completed 3 requests
Completed 4 requests
Completed 5 requests
Completed 6 requests
Completed 7 requests
Completed 8 requests
Completed 9 requests
Completed 10 requests
Finished 10 requests


Server Software:nginx/1.11.3
Server Hostname:127.0.0.1
Server Port:80

Document Path:  /
Document Length:612 bytes

Concurrency Level:  100
Time taken for tests:   10.636 seconds
Complete requests:  10
Failed requests:0
Total transferred:  8450 bytes
HTML transferred:   6120 bytes
Requests per second:9401.97 [#/sec] (mean)
Time per request:   10.636 [ms] (mean)
Time per request:   0.106 [ms] (mean, across all concurrent requests)
Transfer rate:  7758.46 [Kbytes/sec] received

Connection Times (ms)
  min  mean[+/-sd] median   max
Connect:02   1.9  2   8
Processing: 29   2.8  9 206
Waiting:18   3.1  8 205
Total:  6   11   1.6 11 206

Percentage of the requests served within a certain time (ms)
  50% 11
  66% 11
  75% 11
  80% 11
  90% 12
  95% 12
  98% 14
  99% 14
 100%206 (longest request)


repro.sh
Description: Bourne shell script
This is ApacheBench, Version 2.3 <$Revision: 1528965 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking 127.0.0.1 (be patient)
Completed 1 requests
Completed 2 requests
Completed 3 requests
Completed 4 requests
Completed 5 requests
Completed 6 requests
Completed 7 requests
Completed 8 requests
Completed 9 requests
Completed 10 requests
Finished 10 requests


Server Software:nginx/1.11.3
Server Hostname:127.0.0.1
Server Port:80

Document Path:  /
Document Length:612 bytes

Concurrency Level:  100
Time taken for tests:   7.423 seconds
Complete requests:  10
Failed requests:0
Total transferred:  8450 bytes
HTML transferred:   6120 bytes
Requests per second:13471.01 [#/sec] (mean)
Time per request:   7.423 [ms] (mean)
Time per request:   0.074 [ms] (mean, across all concurrent requests)
Transfer rate:  6.21 [Kbytes/sec] received

Connection Times (ms)
  min  mean[+/-sd] median   max
Connect:00   0.5  0   5
Processing: 17   0.9  7  12
Waiting:17   0.9  7  12
Total:  47   0.6  7  12

Percentage of the requests served within a certain time (ms)
  50%  7
  66%  7
  75%  7
  80%  7
  90%  7
  95%  8

Horrid balance-rr bonding udp throughput

[Jarod Wilson]

I'm digging into some bug reports covering performance issues with 
balance-rr, and discovered something even worse than the reporter. My 
test setup has a pair of NICs, one e1000e, one e1000 (but dual e1000e 
seems the same). When I do a test run in LNST with bonding mode 
balance-rr and either miimon or arpmon, the throughput of the UDP_STREAM 
netperf test is absolutely horrible:


TCP: 941.19 +-0.88 mbits/sec
UDP: 45.42 +-4.59 mbits/sec

I figured I'd try LNST's packet capture mode, so exact same test, add 
the -p flag and I get:


TCP: 941.21 +-0.82 mbits/sec
UDP: 961.54 +-0.01 mbits/sec

Uh. What? So yeah. I can't capture the traffic in the bad case, but I 
guess that gives some potential insight into what's not happening 
correctly in either the bonding driver or the NIC drivers... More 
digging forthcoming, but first I have a flooded basement to deal with, 
so if in the interim, anyone has some insight, I'd be happy to hear it. :)


--
Jarod Wilson
ja...@redhat.com

Re: [RFC net-next] bpf: taint loading !is_gpl programs

[Alexei Starovoitov]

On Fri, Apr 07, 2017 at 01:46:28PM -0400, Aaron Conole wrote:
> Hi Alexei, and Daniel,
> 
> Alexei Starovoitov  writes:
> 
> > On Wed, Apr 05, 2017 at 10:59:49PM -0400, Aaron Conole wrote:
> >> Hi Daniel,
> >> 
> >> Daniel Borkmann  writes:
> >> 
> >> > On 04/04/2017 08:33 PM, Aaron Conole wrote:
> >> >> The eBPF framework is used for more than just socket level filtering.  
> >> >> It
> >> >> can also provide tracing, and even change the way packets coming into 
> >> >> the
> >> >> system look.  Most of the eBPF callable symbols are available to non-gpl
> >> >> programs, and this includes helper functions which modify packets.  This
> >> >> allows proprietary eBPF code to link to the kernel and make decisions
> >> >> which can negatively impact network performance.
> >> >>
> >> >> Since the sources for these programs are only available under a 
> >> >> proprietary
> >> >> license, it seems better to treat them the same as other proprietary
> >> >> modules: set the system taint flag.  An exemption is made for 
> >> >> socket-level
> >> >> filters, since they do not really impact networking for the whole 
> >> >> kernel.
> >> >>
> >> >> Signed-off-by: Aaron Conole 
> >> >> ---
> >> >
> >> > Nacked-by: Daniel Borkmann 
> 
> Given we have different views about this, I think I am okay with some
> middle ground.
> 
> Here's the next-steps plan.  Please tell if you dislike it or want to
> change it:
> 
> 1. Add a ref counter for tracking load and unload, which can be queried
> from a procfs or bpf fs interface
> 
> 2. Add a new print during panic when the refcount is non-zero.
> 
> This lets us know that there could be some kind of ebpf program loaded,
> and we would ask for sources before trying to disassemble.
> 
> Does this sound reasonable?

yeah. for the purpose of identifying whether any classic or extended bpf
programs loaded that makes sense. The only question how far we should
take it, since nft and acpi bytecode falls into the same category.
Also my understanding this is just one of out of many items on
redhat todo list to make bpf supported in rhel, so I think it makes
sense to discuss the whole list all at once. If we add patches here and
there without having full picture we may end up with obsolete api
or api superseded by other patches/features.
So let's discuss all feature requests first.

Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor

[David Ahern]

On 4/4/17 9:25 AM, Vladislav Yasevich wrote:
> Add IFLA_EVENT handling so that event types can be viewed with
> 'monitor' command.  This gives a little more information for why
> a given message was receivied.
> 
> Signed-off-by: Vladislav Yasevich 
> ---
>  include/linux/if_link.h | 21 +
>  ip/ipaddress.c  | 31 +++
>  2 files changed, 52 insertions(+)

per comments on the email thread about reducing notifications, the
kernel patch for this should be reverted (and hence this iproute2 patch
is not needed) in favor of using a bitmask. Right now there are too many
redundant notifications to userspace.

Vlad: can you look at a bitmask version of IFLA_EVENT that shows what
changed in the newlink message from do_setlink()?

Re: [PATCH] net: netfilter: ipvs: Replace explicit NULL comparison

[kbuild test robot]

Hi Arushi,

[auto build test WARNING on ipvs-next/master]
[also build test WARNING on v4.11-rc5 next-20170407]
[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/Arushi-Singhal/net-netfilter-ipvs-Replace-explicit-NULL-comparison/20170409-044710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git 
master
config: i386-randconfig-x002-201715 (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=i386 

All warnings (new ones prefixed by >>):

   net/netfilter/ipvs/ip_vs_proto.c: In function 'ip_vs_protocol_net_cleanup':
>> net/netfilter/ipvs/ip_vs_proto.c:350:3: warning: suggest parentheses around 
>> assignment used as truth value [-Wparentheses]
  while (pd = ipvs->proto_data_table[i])
  ^
   net/netfilter/ipvs/ip_vs_proto.c: In function 'ip_vs_protocol_cleanup':
   net/netfilter/ipvs/ip_vs_proto.c:395:3: warning: suggest parentheses around 
assignment used as truth value [-Wparentheses]
  while (pp = ip_vs_proto_table[i])
  ^

vim +350 net/netfilter/ipvs/ip_vs_proto.c

   334  goto cleanup;
   335  }
   336  return 0;
   337  
   338  cleanup:
   339  ip_vs_protocol_net_cleanup(ipvs);
   340  return ret;
   341  }
   342  
   343  void __net_exit ip_vs_protocol_net_cleanup(struct netns_ipvs *ipvs)
   344  {
   345  struct ip_vs_proto_data *pd;
   346  int i;
   347  
   348  /* unregister all the ipvs proto data for this netns */
   349  for (i = 0; i < IP_VS_PROTO_TAB_SIZE; i++) {
 > 350  while (pd = ipvs->proto_data_table[i])
   351  unregister_ip_vs_proto_netns(ipvs, pd);
   352  }
   353  }
   354  
   355  int __init ip_vs_protocol_init(void)
   356  {
   357  char protocols[64];
   358  #define REGISTER_PROTOCOL(p)\

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


.config.gz
Description: application/gzip

Re: [PATCH net-next] bpf: fix comment typo

[Daniel Borkmann]

On 04/08/2017 10:08 PM, Alexander Alemayhu wrote:

o s/bpf_bpf_get_socket_cookie/bpf_get_socket_cookie

Signed-off-by: Alexander Alemayhu 


Acked-by: Daniel Borkmann 

[PATCH] net: rfkill: gpio: Add OBDA8723 ACPI HID

[Hans de Goede]

The OBDA8723 ACPI HID is used on quite a few Bay Trail based tablets
for bluetooth rfkill functionality.

Tested-by: russianneuroman...@ya.ru 
Signed-off-by: Hans de Goede 
---
 net/rfkill/rfkill-gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 76c01cb..50ca65e 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -163,6 +163,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
 static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
{ "LNV4752", RFKILL_TYPE_GPS },
+   { "OBDA8723", RFKILL_TYPE_BLUETOOTH },
{ },
 };
 MODULE_DEVICE_TABLE(acpi, rfkill_acpi_match);
-- 
2.9.3

Re: [PATCH net-next v2 0/3] net: dsa: Receive path simplifications

[David Miller]

From: Florian Fainelli 
Date: Sat,  8 Apr 2017 08:55:20 -0700

> This patch series does factor the common code found in all tag implementations
> into dsa_switch_rcv(). The original motivation was to add GRO support, but 
> this
> may be a lot of work with unclear benefits at this point.
> 
> Changes in v2:
> - take care of tag_mtk.c in the process

Series applied, thanks.

Non-standard TCP stack processing of packets with unacceptable ACK numbers

[Paul Fiterau Brostean]

Hello,

My name is Paul Fiterau, I am a PhD student at Radboud University whose 
focus for the past few years has been among others to develop and apply 
inference techniques on TCP stacks in order to obtain nice models, and 
to verify them if possible using formal methods.  We contacted you on 
something similar 2 years back.


The older (3.19 kernel release) Linux TCP stack we analyze exhibits 
behavior that seems odd to me. The scenario is as follows (all packets 
have empty payloads, no window scaling, rcv/snd window size should not 
be a factor):


  TEST HARNESS (CLIENT)LINUX SERVER

  1.  -  LISTEN (server listen, then 
accepts)

  2.  - --> 

[PATCH v3 2/5] genetlink: pass extended ACK report down

[Johannes Berg]

From: Johannes Berg 

Pass the extended ACK reporting struct down from
generic netlink to the families, using the existing
struct genl_info for simplicity.

Also add support to set the extended ACK information
from generic netlink users.

Signed-off-by: Johannes Berg 
---
 include/net/genetlink.h | 20 
 net/netlink/genetlink.c |  6 --
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index a34275be3600..b81a4979e1db 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -84,6 +84,7 @@ struct nlattr **genl_family_attrbuf(const struct genl_family 
*family);
  * @attrs: netlink attributes
  * @_net: network namespace
  * @user_ptr: user pointers
+ * @extack: extended ACK report struct
  */
 struct genl_info {
u32 snd_seq;
@@ -94,6 +95,7 @@ struct genl_info {
struct nlattr **attrs;
possible_net_t  _net;
void *  user_ptr[2];
+   struct netlink_ext_ack *extack;
 };
 
 static inline struct net *genl_info_net(struct genl_info *info)
@@ -106,6 +108,24 @@ static inline void genl_info_net_set(struct genl_info 
*info, struct net *net)
write_pnet(>_net, net);
 }
 
+#define GENL_SET_ERR_MSG(info, msg) NL_SET_ERR_MSG((info)->extack, msg)
+
+static inline int genl_err_attr(struct genl_info *info, int err,
+   struct nlattr *attr)
+{
+   info->extack->bad_attr = attr;
+
+   return err;
+}
+
+static inline int genl_err_attr_missing(struct genl_info *info, int err,
+   u16 attr)
+{
+   info->extack->missing_attr = attr;
+
+   return err;
+}
+
 /**
  * struct genl_ops - generic netlink operations
  * @cmd: command identifier
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 57b2e3648bc0..4b598a5999a2 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -497,7 +497,8 @@ static int genl_lock_done(struct netlink_callback *cb)
 
 static int genl_family_rcv_msg(const struct genl_family *family,
   struct sk_buff *skb,
-  struct nlmsghdr *nlh)
+  struct nlmsghdr *nlh,
+  struct netlink_ext_ack *extack)
 {
const struct genl_ops *ops;
struct net *net = sock_net(skb->sk);
@@ -584,6 +585,7 @@ static int genl_family_rcv_msg(const struct genl_family 
*family,
info.genlhdr = nlmsg_data(nlh);
info.userhdr = nlmsg_data(nlh) + GENL_HDRLEN;
info.attrs = attrbuf;
+   info.extack = extack;
genl_info_net_set(, net);
memset(_ptr, 0, sizeof(info.user_ptr));
 
@@ -618,7 +620,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh,
if (!family->parallel_ops)
genl_lock();
 
-   err = genl_family_rcv_msg(family, skb, nlh);
+   err = genl_family_rcv_msg(family, skb, nlh, extack);
 
if (!family->parallel_ops)
genl_unlock();
-- 
2.11.0

[PATCH v3 5/5] netlink: pass extended ACK struct where available

[Johannes Berg]

From: Johannes Berg 

This is an add-on to the previous patch that passes
the extended ACK structure where it's already available
by existing genl_info or extack function arguments.

This was done with this spatch (with some manual
adjustment of indentation):

@@
expression A, B, C, D, E;
identifier fn, info;
@@
fn(..., struct genl_info *info, ...) {
...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, info->extack)
...
}

@@
expression A, B, C, D, E;
identifier fn, info;
@@
fn(..., struct genl_info *info, ...) {
<...
-nla_parse_nested(A, B, C, D, NULL)
+nla_parse_nested(A, B, C, D, info->extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_parse(A, B, C, D, E, NULL)
+nla_parse(A, B, C, D, E, extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, extack)
...
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_parse_nested(A, B, C, D, NULL)
+nla_parse_nested(A, B, C, D, extack)
...>
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nlmsg_validate(A, B, C, D, NULL)
+nlmsg_validate(A, B, C, D, extack)
...>
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_validate(A, B, C, D, NULL)
+nla_validate(A, B, C, D, extack)
...>
}

@@
expression A, B, C;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_validate_nested(A, B, C, NULL)
+nla_validate_nested(A, B, C, extack)
...>
}

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c   |  2 +-
 drivers/net/team/team.c|  3 ++-
 net/ieee802154/nl802154.c  | 10 +-
 net/netfilter/ipvs/ip_vs_ctl.c |  2 +-
 net/netfilter/nfnetlink.c  |  2 +-
 net/netlink/genetlink.c|  2 +-
 net/nfc/netlink.c  |  2 +-
 net/tipc/bearer.c  | 14 +++---
 net/tipc/net.c |  2 +-
 net/tipc/node.c|  8 
 net/wireless/nl80211.c | 33 ++---
 net/xfrm/xfrm_user.c   |  2 +-
 12 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index fc79906c1fe7..b5758768920b 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -523,7 +523,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh,
}
 
err = nlmsg_parse(nlh, crypto_msg_min[type], attrs, CRYPTOCFGA_MAX,
- crypto_policy, NULL);
+ crypto_policy, extack);
if (err < 0)
return err;
 
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 86f227124ba1..65c056e2f705 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2471,7 +2471,8 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, 
struct genl_info *info)
goto team_put;
}
err = nla_parse_nested(opt_attrs, TEAM_ATTR_OPTION_MAX,
-  nl_option, team_nl_option_policy, NULL);
+  nl_option, team_nl_option_policy,
+  info->extack);
if (err)
goto team_put;
if (!opt_attrs[TEAM_ATTR_OPTION_NAME] ||
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index d6b1a1b21909..99f6c254ea77 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -1564,7 +1564,7 @@ static int nl802154_add_llsec_key(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, NL802154_KEY_ATTR_MAX,
 info->attrs[NL802154_ATTR_SEC_KEY],
-nl802154_key_policy, NULL))
+nl802154_key_policy, info->extack))
return -EINVAL;
 
if (!attrs[NL802154_KEY_ATTR_USAGE_FRAMES] ||
@@ -1614,7 +1614,7 @@ static int nl802154_del_llsec_key(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, NL802154_KEY_ATTR_MAX,
 info->attrs[NL802154_ATTR_SEC_KEY],
-nl802154_key_policy, NULL))
+nl802154_key_policy, info->extack))
return -EINVAL;
 
if (ieee802154_llsec_parse_key_id(attrs[NL802154_KEY_ATTR_ID], ) < 0)
@@ -1782,7 +1782,7 @@ static int nl802154_del_llsec_dev(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, 

Re: [PATCH net] netfilter: xt_TCPMSS: add more sanity tests on tcph->doff

[Pablo Neira Ayuso]

On Mon, Apr 03, 2017 at 10:55:11AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet 
> 
> Denys provided an awesome KASAN report pointing to an use
> after free in xt_TCPMSS
> 
> I have provided three patches to fix this issue, either in xt_TCPMSS or
> in xt_tcpudp.c. It seems xt_TCPMSS patch has the smallest possible
> impact.

Applied to nf.git, thanks!

[PATCH v3 3/5] netlink: allow sending extended ACK with cookie on success

[Johannes Berg]

From: Johannes Berg 

Now that we have extended error reporting and a new message format
for netlink ACK messages, also extend this to be able to return
arbitrary cookie data on success.

This will allow, for example, nl80211 to not send an extra message
for cookies identifying newly created objects, but return those
directly in the ACK message.

The cookie data size is currently limited to 20 bytes (since Jamal
talked about using SHA1 for identifiers.)

Thanks to Jamal Hadi Salim for bringing up this idea during the
discussions.

Signed-off-by: Johannes Berg 
---
 include/linux/netlink.h  |  7 +++
 include/uapi/linux/netlink.h |  4 
 net/netlink/af_netlink.c | 38 --
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 746359c27396..4bf13bb725ea 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -62,17 +62,24 @@ netlink_kernel_create(struct net *net, int unit, struct 
netlink_kernel_cfg *cfg)
return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
 }
 
+/* this can be increased when necessary - don't expose to userland */
+#define NETLINK_MAX_COOKIE_LEN 20
+
 /**
  * struct netlink_ext_ack - netlink extended ACK report struct
  * @_msg: message string to report - don't access directly, use
  * %NL_SET_ERR_MSG
  * @bad_attr: attribute with error
  * @missing_attr: number of missing attr (or 0)
+ * @cookie: cookie data to return to userspace (for success)
+ * @cookie_len: actual cookie data length
  */
 struct netlink_ext_ack {
const char *_msg;
const struct nlattr *bad_attr;
u16 missing_attr;
+   u8 cookie[NETLINK_MAX_COOKIE_LEN];
+   u8 cookie_len;
 };
 
 /* Always use this macro, this allows later putting the
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 5f1a48b140c8..26625355ac97 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -118,6 +118,9 @@ struct nlmsgerr {
  * @NLMSGERR_ATTR_OFFS: error offset in the original message (u32)
  * @NLMSGERR_ATTR_ATTR: top-level attribute that caused the error
  * (or is missing, u16)
+ * @NLMSGERR_ATTR_COOKIE: arbitrary subsystem specific cookie to
+ * be used - in the success case - to identify a created
+ * object or operation or similar (binary)
  * @NUM_NLMSGERR_ATTRS: number of attributes
  * @NLMSGERR_ATTR_MAX: highest attribute number
  */
@@ -126,6 +129,7 @@ enum nlmsgerr_attrs {
NLMSGERR_ATTR_MSG,
NLMSGERR_ATTR_OFFS,
NLMSGERR_ATTR_ATTR,
+   NLMSGERR_ATTR_COOKIE,
 
NUM_NLMSGERR_ATTRS,
NLMSGERR_ATTR_MAX = NUM_NLMSGERR_ATTRS - 1
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c74f56a4fcf1..78f33b8011d2 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2311,6 +2311,9 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr 
*nlh, int err,
if (extack->missing_attr)
acksize += nla_total_size(sizeof(u16));
}
+   } else if (nlk->flags & NETLINK_F_EXT_ACK) {
+   if (extack && extack->cookie_len)
+   acksize += nla_total_size(extack->cookie_len);
}
 
skb = nlmsg_new(acksize, GFP_KERNEL);
@@ -2336,20 +2339,27 @@ void netlink_ack(struct sk_buff *in_skb, struct 
nlmsghdr *nlh, int err,
   !(nlk->flags & NETLINK_F_CAP_ACK) ? nlh->nlmsg_len
 : sizeof(*nlh));
 
-   if (err && nlk->flags & NETLINK_F_EXT_ACK && extack) {
-   if (extack->_msg)
-   WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
-  extack->_msg));
-   if (extack->bad_attr &&
-   !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
-(u8 *)extack->bad_attr >= in_skb->data +
-  in_skb->len))
-   WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
-   (u8 *)extack->bad_attr -
-   in_skb->data));
-   if (extack->missing_attr)
-   WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
-   extack->missing_attr));
+   if (nlk->flags & NETLINK_F_EXT_ACK && extack) {
+   if (err) {
+   if (extack->_msg)
+   WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
+  extack->_msg));
+   if (extack->bad_attr &&
+   !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
+(u8 *)extack->bad_attr >= in_skb->data +
+   

[PATCH v3 1/5] netlink: extended ACK reporting

[Johannes Berg]

From: Johannes Berg 

Add the base infrastructure and UAPI for netlink
extended ACK reporting. All "manual" calls to
netlink_ack() pass NULL for now and thus don't
get extended ACK reporting.

Big thanks goes to Pablo Neira Ayuso for not only
bringing up the whole topic at netconf (again) but
also coming up with the nlattr passing trick and
various other ideads.

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c  |  3 +-
 drivers/infiniband/core/netlink.c |  5 +--
 drivers/scsi/scsi_netlink.c   |  2 +-
 include/linux/netlink.h   | 28 ++-
 include/net/netlink.h |  3 +-
 include/uapi/linux/netlink.h  | 29 
 kernel/audit.c|  2 +-
 net/core/rtnetlink.c  |  3 +-
 net/core/sock_diag.c  |  3 +-
 net/decnet/netfilter/dn_rtmsg.c   |  2 +-
 net/hsr/hsr_netlink.c |  4 +--
 net/netfilter/ipset/ip_set_core.c |  2 +-
 net/netfilter/nfnetlink.c | 22 ++--
 net/netlink/af_netlink.c  | 71 ++-
 net/netlink/af_netlink.h  |  1 +
 net/netlink/genetlink.c   |  3 +-
 net/xfrm/xfrm_user.c  |  3 +-
 17 files changed, 152 insertions(+), 34 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index a90404a0c5ff..4a44830741c1 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -483,7 +483,8 @@ static const struct crypto_link {
[CRYPTO_MSG_DELRNG  - CRYPTO_MSG_BASE] = { .doit = crypto_del_rng },
 };
 
-static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
+static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+  struct netlink_ext_ack *extack)
 {
struct nlattr *attrs[CRYPTOCFGA_MAX+1];
const struct crypto_link *link;
diff --git a/drivers/infiniband/core/netlink.c 
b/drivers/infiniband/core/netlink.c
index 10469b0088b5..b784055423c8 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -146,7 +146,8 @@ int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 EXPORT_SYMBOL(ibnl_put_attr);
 
-static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
+static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+   struct netlink_ext_ack *extack)
 {
struct ibnl_client *client;
int type = nlh->nlmsg_type;
@@ -209,7 +210,7 @@ static void ibnl_rcv_reply_skb(struct sk_buff *skb)
if (nlh->nlmsg_flags & NLM_F_REQUEST)
return;
 
-   ibnl_rcv_msg(skb, nlh);
+   ibnl_rcv_msg(skb, nlh, NULL);
 
msglen = NLMSG_ALIGN(nlh->nlmsg_len);
if (msglen > skb->len)
diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
index 109802f776ed..50e624fb8307 100644
--- a/drivers/scsi/scsi_netlink.c
+++ b/drivers/scsi/scsi_netlink.c
@@ -111,7 +111,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 
 next_msg:
if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
-   netlink_ack(skb, nlh, err);
+   netlink_ack(skb, nlh, err, NULL);
 
skb_pull(skb, rlen);
}
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index da14ab61f363..746359c27396 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -62,11 +62,37 @@ netlink_kernel_create(struct net *net, int unit, struct 
netlink_kernel_cfg *cfg)
return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
 }
 
+/**
+ * struct netlink_ext_ack - netlink extended ACK report struct
+ * @_msg: message string to report - don't access directly, use
+ * %NL_SET_ERR_MSG
+ * @bad_attr: attribute with error
+ * @missing_attr: number of missing attr (or 0)
+ */
+struct netlink_ext_ack {
+   const char *_msg;
+   const struct nlattr *bad_attr;
+   u16 missing_attr;
+};
+
+/* Always use this macro, this allows later putting the
+ * message into a separate section or such for things
+ * like translation or listing all possible messages.
+ * Currently string formatting is not supported (due
+ * to the lack of an output buffer.)
+ */
+#define NL_SET_ERR_MSG(extack, msg) do {   \
+   static const char *_msg = (msg);\
+   \
+   (extack)->_msg = _msg;  \
+} 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);
 extern void __netlink_clear_multicast_users(struct sock *sk, unsigned int 
group);
-extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
+extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
+   const struct netlink_ext_ack 

[PATCH v3 4/5] netlink: pass extended ACK struct to parsing functions

[Johannes Berg]

From: Johannes Berg 

Pass the new extended ACK reporting struct to all of the
generic netlink parsing functions. For now, pass NULL in
almost all callers (except for some in the core.)

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c  |  2 +-
 drivers/block/drbd/drbd_nla.c |  2 +-
 drivers/infiniband/core/addr.c|  2 +-
 drivers/infiniband/core/iwpm_util.c   |  6 +-
 drivers/infiniband/core/sa_query.c|  4 +-
 drivers/net/macsec.c  | 10 +--
 drivers/net/team/team.c   |  2 +-
 drivers/net/veth.c|  3 +-
 drivers/net/wireless/ath/ath10k/testmode.c|  4 +-
 drivers/net/wireless/ath/ath6kl/testmode.c|  4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c |  3 +-
 drivers/net/wireless/mac80211_hwsim.c |  4 +-
 drivers/net/wireless/marvell/mwifiex/cfg80211.c   |  4 +-
 drivers/net/wireless/ti/wlcore/testmode.c |  3 +-
 drivers/net/wireless/ti/wlcore/vendor_cmd.c   |  4 +-
 include/net/genetlink.h   |  8 ++-
 include/net/netlink.h | 33 +++---
 include/net/rtnetlink.h   |  3 +-
 lib/nlattr.c  | 28 +---
 net/8021q/vlan_netlink.c  |  3 +-
 net/bridge/br_mdb.c   |  3 +-
 net/bridge/br_netlink.c   |  4 +-
 net/bridge/br_netlink_tunnel.c|  4 +-
 net/can/gw.c  |  2 +-
 net/core/fib_rules.c  |  4 +-
 net/core/lwt_bpf.c|  5 +-
 net/core/neighbour.c  |  8 +--
 net/core/net_namespace.c  |  4 +-
 net/core/rtnetlink.c  | 47 --
 net/dcb/dcbnl.c   | 57 -
 net/decnet/dn_dev.c   |  4 +-
 net/decnet/dn_fib.c   |  6 +-
 net/decnet/dn_route.c |  2 +-
 net/ieee802154/nl802154.c | 29 -
 net/ipv4/devinet.c| 12 ++--
 net/ipv4/fib_frontend.c   |  3 +-
 net/ipv4/ip_tunnel_core.c |  5 +-
 net/ipv4/ipmr.c   |  3 +-
 net/ipv4/route.c  |  3 +-
 net/ipv6/addrconf.c   | 16 +++--
 net/ipv6/addrlabel.c  |  4 +-
 net/ipv6/ila/ila_lwt.c|  3 +-
 net/ipv6/route.c  |  6 +-
 net/ipv6/seg6_iptunnel.c  |  2 +-
 net/mpls/af_mpls.c|  5 +-
 net/mpls/mpls_iptunnel.c  |  2 +-
 net/netfilter/ipset/ip_set_core.c | 27 
 net/netfilter/ipvs/ip_vs_ctl.c| 12 ++--
 net/netfilter/nf_conntrack_netlink.c  | 27 
 net/netfilter/nf_conntrack_proto_dccp.c   |  2 +-
 net/netfilter/nf_conntrack_proto_sctp.c   |  6 +-
 net/netfilter/nf_conntrack_proto_tcp.c|  3 +-
 net/netfilter/nf_nat_core.c   |  5 +-
 net/netfilter/nf_tables_api.c | 27 
 net/netfilter/nfnetlink.c | 11 ++--
 net/netfilter/nfnetlink_acct.c|  3 +-
 net/netfilter/nfnetlink_cthelper.c| 12 ++--
 net/netfilter/nfnetlink_cttimeout.c   |  3 +-
 net/netfilter/nfnetlink_queue.c   |  2 +-
 net/netfilter/nft_compat.c|  2 +-
 net/netlabel/netlabel_cipso_v4.c  | 19 +++---
 net/netlink/genetlink.c   |  2 +-
 net/nfc/netlink.c |  5 +-
 net/openvswitch/datapath.c|  2 +-
 net/openvswitch/flow_netlink.c|  4 +-
 net/openvswitch/vport-vxlan.c |  3 +-
 net/phonet/pn_netlink.c   |  6 +-
 net/qrtr/qrtr.c   |  2 +-
 net/sched/act_api.c   | 20 +++---
 net/sched/act_bpf.c   |  2 +-
 net/sched/act_connmark.c  |  3 +-
 net/sched/act_csum.c  |  2 +-
 net/sched/act_gact.c  |  2 +-
 net/sched/act_ife.c   |  4 +-
 net/sched/act_ipt.c   |  2 +-
 net/sched/act_mirred.c|  2 +-
 net/sched/act_nat.c   |  2 +-
 net/sched/act_pedit.c |  4 +-
 net/sched/act_police.c   

[PATCH v3 0/5] netlink extended ACK reporting

[Johannes Berg]

Changes since v2:
 * add NUM_NLMSGERR_ATTRS, NLMSGERR_ATTR_MAX
 * fix cookie length to 20 (sha-1 length)
 * move struct members for cookie to patch 3 where they should be
 * another cleanup suggested by David Ahern

johannes

Re: [PATCH 4/5] netlink: pass extended ACK struct to parsing functions

[Johannes Berg]

On Sat, 2017-04-08 at 14:50 -0400, David Ahern wrote:
> On 4/8/17 1:48 PM, Johannes Berg wrote:
> > From: Johannes Berg 
> > 
> > Pass the new extended ACK reporting struct to all of the
> > generic netlink parsing functions. For now, pass NULL in
> > almost all callers (except for some in the core.)
> > 
> > Signed-off-by: Johannes Berg 
> 
> fails to compile as well:
> 
> In file included from /home/dsa/kernel-
> 4.git/net/sched/cls_rsvp.c:27:0:
> /home/dsa/kernel-4.git/net/sched/cls_rsvp.h: In function
> ‘rsvp_change’:
> /home/dsa/kernel-4.git/net/sched/cls_rsvp.h:487:8: error: too few
> arguments to function ‘nla_parse_nested’
>   err = nla_parse_nested(tb, TCA_RSVP_MAX, opt, rsvp_policy);
> ^

Yeah, spatch misses header files without special command-line arguments
... this should be fixed in v2, the kbuild bot also noticed it.

johannes

Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

[Pablo Neira Ayuso]

On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote:
> On Saturday 2017-04-08 19:21, Arushi Singhal wrote:
> 
> >Replace explicit NULL comparison with ! operator to simplify code.
> 
> I still wouldn't do this, for the same reason as before. Comparing to 
> NULL explicitly more or less gave an extra guarantee that the other 
> operand was also a pointer.

Arushi, where does it say in the coding style that this is prefered?

Re: [PATCH 1/5] netlink: extended ACK reporting

[David Ahern]

On 4/8/17 1:48 PM, Johannes Berg wrote:
> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
> index f3946a27bd07..d1564557d645 100644
> --- a/include/uapi/linux/netlink.h
> +++ b/include/uapi/linux/netlink.h
> @@ -101,6 +101,29 @@ struct nlmsghdr {
>  struct nlmsgerr {
>   int error;
>   struct nlmsghdr msg;
> + /*
> +  * followed by the message contents unless NETLINK_CAP_ACK was set,
> +  * message length is aligned with NLMSG_ALIGN()
> +  */
> + /*
> +  * followed by TLVs defined in enum nlmsgerr_attrs
> +  * if NETLINK_EXT_ACK was set
> +  */
> +};
> +
> +/**
> + * enum nlmsgerr_attrs - netlink error message attributes
> + * @NLMSGERR_ATTR_UNUSED: unused
> + * @NLMSGERR_ATTR_MSG: error message string (string)
> + * @NLMSGERR_ATTR_OFFS: error offset in the original message (u32)
> + * @NLMSGERR_ATTR_ATTR: top-level attribute that caused the error
> + *   (or is missing, u16)
> + */
> +enum nlmsgerr_attrs {
> + NLMSGERR_ATTR_UNUSED,
> + NLMSGERR_ATTR_MSG,
> + NLMSGERR_ATTR_OFFS,
> + NLMSGERR_ATTR_ATTR,


can you add __NLMSGERR_ATTR_MAX and
#define NLMSGERR_ATTR_MAX (__NLMSGERR_ATTR_MAX - 1)

Re: [PATCH 1/5] netlink: extended ACK reporting

[Johannes Berg]

On Sat, 2017-04-08 at 20:40 +0200, Jiri Pirko wrote:

> > I think I'll leave it like this - if anyone really wants to say
> > "attribute 0 is missing" then we can add a flag later... The UAPI
> > does
> > take this into account by not including the attribute at all if the
> > data is invalid, so 0 in the userspace API can be done
> 
> It a known issue, should be fixed right now. We are in no hurry. This
> waited +15 years to be done, no harm in couple more days.

It's not about any timing or anything - I simply think the likelihood
that this will be needed is zero, because almost no netlink family uses
attribute zero, those that do use it are older ones unlikely to be
updated to start with, and *then* needing to indicate that attribute 0
is missing? Not going to happen.

The extra code needed to handle this is therefore wasted.

> Also, could you please attach a patch to iproute2 for example which
> would make use of this. I just want to make sure it clicks.

No, I'm not going to do that. If you want it, please do it yourself.
I've done the testing on a slightly modified iw, but even there haven't
done any pretty-printing or parsing, just made sure the attributes show
up properly (by dumping them in hex).

johannes

Re: [PATCH] net: ipv6: Remove unneccessary comments

[Pablo Neira Ayuso]

On Sat, Apr 08, 2017 at 09:19:30PM +0530, Arushi Singhal wrote:
> This comments are obsolete and should go, as there are no set of rules per
> CPU anymore.

Applied, thanks.

[PATCH net-next] bpf: fix comment typo

[Alexander Alemayhu]

o s/bpf_bpf_get_socket_cookie/bpf_get_socket_cookie

Signed-off-by: Alexander Alemayhu 
---
 include/uapi/linux/bpf.h   | 2 +-
 tools/include/uapi/linux/bpf.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a1d95386f562..1e062bb54eec 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -472,7 +472,7 @@ union bpf_attr {
  *   > 0 length of the string including the trailing NUL on success
  *   < 0 error
  *
- * u64 bpf_bpf_get_socket_cookie(skb)
+ * u64 bpf_get_socket_cookie(skb)
  * Get the cookie for the socket stored inside sk_buff.
  * @skb: pointer to skb
  * Return: 8 Bytes non-decreasing number on success or 0 if the socket
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a1d95386f562..1e062bb54eec 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -472,7 +472,7 @@ union bpf_attr {
  *   > 0 length of the string including the trailing NUL on success
  *   < 0 error
  *
- * u64 bpf_bpf_get_socket_cookie(skb)
+ * u64 bpf_get_socket_cookie(skb)
  * Get the cookie for the socket stored inside sk_buff.
  * @skb: pointer to skb
  * Return: 8 Bytes non-decreasing number on success or 0 if the socket
-- 
2.7.4

[PATCH net-next v2] net: allow configuring default qdisc

[Stephen Hemminger]

Since 3.12 it has been possible to configure the default queuing
discipline via sysctl. This patch adds ability to configure the
default queue discipline in kernel configuration. This is useful for
environments where configuring the value from userspace is difficult
to manage.

The default is still the same as before (pfifo_fast) and it is
possible to change after kernel init with sysctl. This is similar
to how TCP congestion control works.

Signed-off-by: Stephen Hemminger 
---
v2 - rearrange order of menu items
 use pfifo_fast not pfifo

 net/sched/Kconfig   | 31 +++
 net/sched/sch_api.c |  7 +++
 2 files changed, 38 insertions(+)

diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 403790cce7d2..c1243b8e6465 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -352,6 +352,37 @@ config NET_SCH_PLUG
  To compile this code as a module, choose M here: the
  module will be called sch_plug.
 
+choice
+   prompt "Default queuing discipline"
+   default DEFAULT_PFIFO_FAST
+   help
+ Select the queueing discipline that will be used by default
+ for all network devices.
+
+   config DEFAULT_FQ
+   bool "Fair Queue" if NET_SCH_FQ
+
+   config DEFAULT_CODEL
+   bool "Controlled Delay" if NET_SCH_CODEL
+
+   config DEFAULT_FQ_CODEL
+   bool "Fair Queue Controlled Delay" if NET_SCH_FQ_CODEL
+
+   config DEFAULT_SFQ
+   bool "Stochastic Fair Queue" if NET_SCH_SFQ
+
+   config DEFAULT_PFIFO_FAST
+   bool "Priority FIFO Fast"
+endchoice
+
+config DEFAULT_NET_SCH
+   string
+   default "pfifo_fast" if DEFAULT_PFIFO_FAST
+   default "fq" if DEFAULT_FQ
+   default "fq_codel" if DEFAULT_FQ_CODEL
+   default "sfq" if DEFAULT_SFQ
+   default "pfifo_fast"
+
 comment "Classification"
 
 config NET_CLS
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 62567bfe52c7..0d82f76f622d 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -251,6 +251,13 @@ int qdisc_set_default(const char *name)
return ops ? 0 : -ENOENT;
 }
 
+/* Set default value from kernel config */
+static int __init sch_default_qdisc(void)
+{
+   return qdisc_set_default(CONFIG_DEFAULT_NET_SCH);
+}
+late_initcall(sch_default_qdisc);
+
 /* We know handle. Find qdisc among all qdisc's attached to device
  * (root qdisc, all its children, children of children etc.)
  * Note: caller either uses rtnl or rcu_read_lock()
-- 
2.11.0

Re: [PATCH net-next 0/1 v2] skbuff: Extend gso_type to unsigned int.

[David Miller]

From: Eric Dumazet 
Date: Sat, 08 Apr 2017 11:55:32 -0700

> On Sat, 2017-04-08 at 20:36 +0200, Steffen Klassert wrote:
>> All available gso_type flags are currently in use, so
>> extend gso_type from 'unsigned short' to 'unsigned int'
>> to be able to add further flags.
>> 
>> We reorder the struct skb_shared_info to use
>> two bytes of the four byte hole before dataref.
>> All fields before dataref are cleared, i.e.
>> four bytes more than before the change.
>> 
>> The remaining two byte hole is moved to the
>> beginning of the structure, this protects us
>> from immediate overwites on out of bound writes
>> to the sk_buff head.
> 
>> Signed-off-by: Steffen Klassert 
...
> This looks much better, thanks Steffen.
> 
> Acked-by: Eric Dumazet 

Applied, thanks everyone.

Re: [PATCH net-next] net_sched: allow configuring default qdisc

[Eric Dumazet]

On Sat, 2017-04-08 at 14:50 -0400, Stephen Hemminger wrote:
> Since 3.12 it has been possible to configure the default queuing discipline
> via sysctl. This patch adds ability to configure the default queue discipline
> in kernel configuration. This is useful for environments where configuring
> the value from userspace is difficult to manage.
> 
> The default is still the same as before (pfifo_fast) and it is possible
> to change after kernel init with sysctl.  This is analagous to how
> TCP congestion control is configured.
> 
> Signed-off-by: Stephen Hemminger 
> ---
>  net/sched/Kconfig   | 31 +++
>  net/sched/sch_api.c |  7 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/net/sched/Kconfig b/net/sched/Kconfig
> index 403790cce7d2..8fb45655e59b 100644
> --- a/net/sched/Kconfig
> +++ b/net/sched/Kconfig
> @@ -820,6 +820,37 @@ config NET_CLS_IND
> classification based on the incoming device. This option is
> likely to disappear in favour of the metadata ematch.
>  
> +choice
> + prompt "Default queuing discipline"
> + default DEFAULT_PFIFO
> + help
> +   Select the queueing discipline that will be used by default
> +   for all network devices.
> +
> + config DEFAULT_FQ
> + bool "Fair Queue" if NET_SCH_FQ
> +
> + config DEFAULT_FQ_CODEL
> + bool "Fair Queue Controlled Delay (FQ_CODEL)" if 
> NET_SCH_FQ_CODEL
> +
> + config DEFAULT_CODEL
> + bool "Controlled Delay (CODEL)" if NET_SCH_CODEL
> +
> + config DEFAULT_SFQ
> + bool "Stochastic Fair Queue (SFQ)" if NET_SCH_SFQ
> +
> + config DEFAULT_PFIFO
> + bool "Priority FIFO"
> +endchoice
> +
> +config DEFAULT_NET_SCH
> + string
> + default "pfifo" if DEFAULT_PFIFO
> + default "fq" if DEFAULT_FQ
> + default "fq_codel" if DEFAULT_FQ_CODEL
> + default "sfq" if DEFAULT_SFQ
> + default "pfifo"
> +
>  endif # NET_SCHED


Note that pfifo != pfifo_fast

We probably still want pfifo_fast being the default ?

Re: [PATCH v2 3/5] netlink: allow sending extended ACK with cookie on success

[David Ahern]

since a v3 is coming another small cleanup ...


On 4/8/17 2:34 PM, Johannes Berg wrote:
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index c74f56a4fcf1..98e55a59c97e 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -2336,20 +2339,27 @@ void netlink_ack(struct sk_buff *in_skb, struct 
> nlmsghdr *nlh, int err,
>  !(nlk->flags & NETLINK_F_CAP_ACK) ? nlh->nlmsg_len
>: sizeof(*nlh));
>  
> - if (err && nlk->flags & NETLINK_F_EXT_ACK && extack) {
> - if (extack->_msg)
> - WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
> -extack->_msg));
> - if (extack->bad_attr &&
> - !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
> -  (u8 *)extack->bad_attr >= in_skb->data +
> -in_skb->len))
> - WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
> - (u8 *)extack->bad_attr -
> - in_skb->data));
> - if (extack->missing_attr)
> - WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
> - extack->missing_attr));
> + if (nlk->flags & NETLINK_F_EXT_ACK) {
> + if (err && extack) {

push the extack into the flags if check

> + if (extack->_msg)
> + WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
> +extack->_msg));
> + if (extack->bad_attr &&
> + !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
> +  (u8 *)extack->bad_attr >= in_skb->data +
> +in_skb->len))
> + WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
> + (u8 *)extack->bad_attr -
> + in_skb->data));
> + if (extack->missing_attr)
> + WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
> + extack->missing_attr));
> + } else if (!err && extack) {

and here too

> + if (extack->cookie_len)
> + WARN_ON(nla_put(skb, NLMSGERR_ATTR_COOKIE,
> + extack->cookie_len,
> + extack->cookie));
> + }

XDP Newbies...

[David Miller]

I've created a new mailing list:

xdp-newb...@vger.kernel.org

Which is a place where people can talk about getting up to speed
with setting up an XDP build environment and writing XDP programs.

Please spread the word!

Re: [PATCH net-next 0/1 v2] skbuff: Extend gso_type to unsigned int.

[Eric Dumazet]

On Sat, 2017-04-08 at 20:36 +0200, Steffen Klassert wrote:
> All available gso_type flags are currently in use, so
> extend gso_type from 'unsigned short' to 'unsigned int'
> to be able to add further flags.
> 
> We reorder the struct skb_shared_info to use
> two bytes of the four byte hole before dataref.
> All fields before dataref are cleared, i.e.
> four bytes more than before the change.
> 
> The remaining two byte hole is moved to the
> beginning of the structure, this protects us
> from immediate overwites on out of bound writes
> to the sk_buff head.

> Signed-off-by: Steffen Klassert 
> ---
>  include/linux/skbuff.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index c776abd..741d75c 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -413,14 +413,15 @@ struct ubuf_info {
>   * the end of the header data, ie. at skb->end.
>   */
>  struct skb_shared_info {
> + unsigned short  _unused;
>   unsigned char   nr_frags;
>   __u8tx_flags;
>   unsigned short  gso_size;
>   /* Warning: this field is not always filled in (UFO)! */
>   unsigned short  gso_segs;
> - unsigned short  gso_type;
>   struct sk_buff  *frag_list;
>   struct skb_shared_hwtstamps hwtstamps;
> + unsigned intgso_type;
>   u32 tskey;
>   __be32  ip6_frag_id;
>  

This looks much better, thanks Steffen.

Acked-by: Eric Dumazet 

[PATCH net-next] net_sched: allow configuring default qdisc

[Stephen Hemminger]

Since 3.12 it has been possible to configure the default queuing discipline
via sysctl. This patch adds ability to configure the default queue discipline
in kernel configuration. This is useful for environments where configuring
the value from userspace is difficult to manage.

The default is still the same as before (pfifo_fast) and it is possible
to change after kernel init with sysctl.  This is analagous to how
TCP congestion control is configured.

Signed-off-by: Stephen Hemminger 
---
 net/sched/Kconfig   | 31 +++
 net/sched/sch_api.c |  7 +++
 2 files changed, 38 insertions(+)

diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 403790cce7d2..8fb45655e59b 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -820,6 +820,37 @@ config NET_CLS_IND
  classification based on the incoming device. This option is
  likely to disappear in favour of the metadata ematch.
 
+choice
+   prompt "Default queuing discipline"
+   default DEFAULT_PFIFO
+   help
+ Select the queueing discipline that will be used by default
+ for all network devices.
+
+   config DEFAULT_FQ
+   bool "Fair Queue" if NET_SCH_FQ
+
+   config DEFAULT_FQ_CODEL
+   bool "Fair Queue Controlled Delay (FQ_CODEL)" if 
NET_SCH_FQ_CODEL
+
+   config DEFAULT_CODEL
+   bool "Controlled Delay (CODEL)" if NET_SCH_CODEL
+
+   config DEFAULT_SFQ
+   bool "Stochastic Fair Queue (SFQ)" if NET_SCH_SFQ
+
+   config DEFAULT_PFIFO
+   bool "Priority FIFO"
+endchoice
+
+config DEFAULT_NET_SCH
+   string
+   default "pfifo" if DEFAULT_PFIFO
+   default "fq" if DEFAULT_FQ
+   default "fq_codel" if DEFAULT_FQ_CODEL
+   default "sfq" if DEFAULT_SFQ
+   default "pfifo"
+
 endif # NET_SCHED
 
 config NET_SCH_FIFO
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 62567bfe52c7..0d82f76f622d 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -251,6 +251,13 @@ int qdisc_set_default(const char *name)
return ops ? 0 : -ENOENT;
 }
 
+/* Set default value from kernel config */
+static int __init sch_default_qdisc(void)
+{
+   return qdisc_set_default(CONFIG_DEFAULT_NET_SCH);
+}
+late_initcall(sch_default_qdisc);
+
 /* We know handle. Find qdisc among all qdisc's attached to device
  * (root qdisc, all its children, children of children etc.)
  * Note: caller either uses rtnl or rcu_read_lock()
-- 
2.11.0

Re: [PATCH 4/5] netlink: pass extended ACK struct to parsing functions

[David Ahern]

On 4/8/17 1:48 PM, Johannes Berg wrote:
> From: Johannes Berg 
> 
> Pass the new extended ACK reporting struct to all of the
> generic netlink parsing functions. For now, pass NULL in
> almost all callers (except for some in the core.)
> 
> Signed-off-by: Johannes Berg 

fails to compile as well:

In file included from /home/dsa/kernel-4.git/net/sched/cls_rsvp.c:27:0:
/home/dsa/kernel-4.git/net/sched/cls_rsvp.h: In function ‘rsvp_change’:
/home/dsa/kernel-4.git/net/sched/cls_rsvp.h:487:8: error: too few
arguments to function ‘nla_parse_nested’
  err = nla_parse_nested(tb, TCA_RSVP_MAX, opt, rsvp_policy);
^
In file included from /home/dsa/kernel-4.git/include/net/rtnetlink.h:5:0,
 from /home/dsa/kernel-4.git/include/net/sch_generic.h:12,
 from /home/dsa/kernel-4.git/include/linux/filter.h:20,
 from /home/dsa/kernel-4.git/include/net/sock.h:64,
 from /home/dsa/kernel-4.git/include/net/inet_sock.h:27,
 from /home/dsa/kernel-4.git/include/net/ip.h:30,
 from /home/dsa/kernel-4.git/net/sched/cls_rsvp.c:18:
/home/dsa/kernel-4.git/include/net/netlink.h:754:19: note: declared here
 static inline int nla_parse_nested(struct nlattr *tb[], int maxtype,

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

[David Miller]

From: Sivakumar Krishnasamy 
Date: Fri,  7 Apr 2017 05:57:59 -0400

> Enable largesend and checksum offload for ibmveth configured in trunk mode.
> Added support to SKB frag_list in TX path by skb_linearize'ing such SKBs.
> 
> Signed-off-by: Sivakumar Krishnasamy 

Why is linearization necessary?

It would seem that the gains you get from GRO are nullified by
linearizing the SKB and thus copying all the data around and
allocating buffers.

Finally, all of that new checksumming stuff looks extremely
suspicious.  You have to explain why that is happening and why it
isn't because this driver is doing something incorrectly.

Thanks.

Re: [PATCH 1/5] netlink: extended ACK reporting

[Jiri Pirko]

Sat, Apr 08, 2017 at 08:37:01PM CEST, johan...@sipsolutions.net wrote:
>On Sat, 2017-04-08 at 20:34 +0200, Jiri Pirko wrote:
>> nla_total_size(sizeof(u32));
>> > +  if (extack &&
>> > +  (extack->missing_attr || extack-
>> > >bad_attr))
>> 
>> Attr could be 0, right? I know that on the most of the places 0 is
>> UNSPEC, but I'm pretty sure not everywhere.
>
>Yeah, I guess we can't show a missing attribute of 0 now - bad_attr is
>a pointer so no problem there.
>
>I think I'll leave it like this - if anyone really wants to say
>"attribute 0 is missing" then we can add a flag later... The UAPI does
>take this into account by not including the attribute at all if the
>data is invalid, so 0 in the userspace API can be done

It a known issue, should be fixed right now. We are in no hurry. This
waited +15 years to be done, no harm in couple more days.


Also, could you please attach a patch to iproute2 for example which
would make use of this. I just want to make sure it clicks.

Thanks!

Re: [PATCH 1/5] netlink: extended ACK reporting

[Johannes Berg]

On Sat, 2017-04-08 at 14:36 -0400, David Ahern wrote:
> 
> I think v3 is in your future ...
> 
> /home/dsa/kernel-4.git/include/linux/netlink.h:78:12: error:
> ‘NETLINK_MAX_COOKIE_LEN’ undeclared here (not in a function)
>   u8 cookie[NETLINK_MAX_COOKIE_LEN];
> ^
> 
> it's defined in patch 3; needed in patch 1

Damn. Ok, that'll have to wait until Monday, unless the airport has
useful wifi.

johannes

Re: [PATCH 1/5] netlink: extended ACK reporting

[Johannes Berg]

On Sat, 2017-04-08 at 20:34 +0200, Jiri Pirko wrote:
> nla_total_size(sizeof(u32));
> > +   if (extack &&
> > +   (extack->missing_attr || extack-
> > >bad_attr))
> 
> Attr could be 0, right? I know that on the most of the places 0 is
> UNSPEC, but I'm pretty sure not everywhere.

Yeah, I guess we can't show a missing attribute of 0 now - bad_attr is
a pointer so no problem there.

I think I'll leave it like this - if anyone really wants to say
"attribute 0 is missing" then we can add a flag later... The UAPI does
take this into account by not including the attribute at all if the
data is invalid, so 0 in the userspace API can be done

johannes

[PATCH net-next 0/1 v2] skbuff: Extend gso_type to unsigned int.

[Steffen Klassert]

All available gso_type flags are currently in use, so
extend gso_type from 'unsigned short' to 'unsigned int'
to be able to add further flags.

We reorder the struct skb_shared_info to use
two bytes of the four byte hole before dataref.
All fields before dataref are cleared, i.e.
four bytes more than before the change.

The remaining two byte hole is moved to the
beginning of the structure, this protects us
from immediate overwites on out of bound writes
to the sk_buff head.

Structure layout on x86-64 before the change:

struct skb_shared_info {
unsigned char  nr_frags; /* 0 1 */
__u8   tx_flags; /* 1 1 */
short unsigned int gso_size; /* 2 2 */
short unsigned int gso_segs; /* 4 2 */
short unsigned int gso_type; /* 6 2 */
struct sk_buff *   frag_list;/* 8 8 */
struct skb_shared_hwtstamps hwtstamps;   /*16 8 */
u32tskey;/*24 4 */
__be32 ip6_frag_id;  /*28 4 */
atomic_t   dataref;  /*32 4 */

/* XXX 4 bytes hole, try to pack */

void * destructor_arg;   /*40 8 */
skb_frag_t frags[17];/*48   272 */
/* --- cacheline 5 boundary (320 bytes) --- */

/* size: 320, cachelines: 5, members: 12 */
/* sum members: 316, holes: 1, sum holes: 4 */
};

Structure layout on x86-64 after the change:

struct skb_shared_info {
short unsigned int _unused;  /* 0 2 */
unsigned char  nr_frags; /* 2 1 */
__u8   tx_flags; /* 3 1 */
short unsigned int gso_size; /* 4 2 */
short unsigned int gso_segs; /* 6 2 */
struct sk_buff *   frag_list;/* 8 8 */
struct skb_shared_hwtstamps hwtstamps;   /*16 8 */
unsigned int   gso_type; /*24 4 */
u32tskey;/*28 4 */
__be32 ip6_frag_id;  /*32 4 */
atomic_t   dataref;  /*36 4 */
void * destructor_arg;   /*40 8 */
skb_frag_t frags[17];/*48   272 */
/* --- cacheline 5 boundary (320 bytes) --- */

/* size: 320, cachelines: 5, members: 13 */
};

Signed-off-by: Steffen Klassert 
---
 include/linux/skbuff.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c776abd..741d75c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -413,14 +413,15 @@ struct ubuf_info {
  * the end of the header data, ie. at skb->end.
  */
 struct skb_shared_info {
+   unsigned short  _unused;
unsigned char   nr_frags;
__u8tx_flags;
unsigned short  gso_size;
/* Warning: this field is not always filled in (UFO)! */
unsigned short  gso_segs;
-   unsigned short  gso_type;
struct sk_buff  *frag_list;
struct skb_shared_hwtstamps hwtstamps;
+   unsigned intgso_type;
u32 tskey;
__be32  ip6_frag_id;
 
-- 
2.7.4

Re: [PATCH 1/5] netlink: extended ACK reporting

[David Ahern]

On 4/8/17 1:48 PM, Johannes Berg wrote:
> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> index da14ab61f363..47562e940e9c 100644
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -62,11 +62,41 @@ netlink_kernel_create(struct net *net, int unit, struct 
> netlink_kernel_cfg *cfg)
>   return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
>  }
>  
> +/**
> + * struct netlink_ext_ack - netlink extended ACK report struct
> + * @_msg: message string to report - don't access directly, use
> + *   %NL_SET_ERR_MSG
> + * @bad_attr: attribute with error
> + * @missing_attr: number of missing attr (or 0)
> + * @cookie: cookie data to return to userspace (for success)
> + * @cookie_len: actual cookie data length
> + */
> +struct netlink_ext_ack {
> + const char *_msg;
> + const struct nlattr *bad_attr;
> + u16 missing_attr;
> + u8 cookie[NETLINK_MAX_COOKIE_LEN];
> + u8 cookie_len;
> +};
> +

I think v3 is in your future ...

/home/dsa/kernel-4.git/include/linux/netlink.h:78:12: error:
‘NETLINK_MAX_COOKIE_LEN’ undeclared here (not in a function)
  u8 cookie[NETLINK_MAX_COOKIE_LEN];
^

it's defined in patch 3; needed in patch 1

[PATCH net-next 0/1 v2] skbuff: Extend gso_type to unsigned int

[Steffen Klassert]

We need a GSO type for IPsec ESP to be able to integrate the IPsec
hardware offloading infrastructure. Unfortunately, all gso_type flags
are currently in use. This change extends gso_type from 'unsigned short'
to 'unsigned int'.

Changes from v1:

- Place the remaining two byte hole in front, suggested by
  Eric Dumazet.

- Skipping the memset of the two byte hole is unnecessary
  as it does not matter if we memset two or four bytes more,
  suggested by Alexander Duyck.

[PATCH v2 4/5] netlink: pass extended ACK struct to parsing functions

[Johannes Berg]

From: Johannes Berg 

Pass the new extended ACK reporting struct to all of the
generic netlink parsing functions. For now, pass NULL in
almost all callers (except for some in the core.)

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c  |  2 +-
 drivers/block/drbd/drbd_nla.c |  2 +-
 drivers/infiniband/core/addr.c|  2 +-
 drivers/infiniband/core/iwpm_util.c   |  6 +-
 drivers/infiniband/core/sa_query.c|  4 +-
 drivers/net/macsec.c  | 10 +--
 drivers/net/team/team.c   |  2 +-
 drivers/net/veth.c|  3 +-
 drivers/net/wireless/ath/ath10k/testmode.c|  4 +-
 drivers/net/wireless/ath/ath6kl/testmode.c|  4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c |  3 +-
 drivers/net/wireless/mac80211_hwsim.c |  4 +-
 drivers/net/wireless/marvell/mwifiex/cfg80211.c   |  4 +-
 drivers/net/wireless/ti/wlcore/testmode.c |  3 +-
 drivers/net/wireless/ti/wlcore/vendor_cmd.c   |  4 +-
 include/net/genetlink.h   |  8 ++-
 include/net/netlink.h | 33 +++---
 include/net/rtnetlink.h   |  3 +-
 lib/nlattr.c  | 28 +---
 net/8021q/vlan_netlink.c  |  3 +-
 net/bridge/br_mdb.c   |  3 +-
 net/bridge/br_netlink.c   |  4 +-
 net/bridge/br_netlink_tunnel.c|  4 +-
 net/can/gw.c  |  2 +-
 net/core/fib_rules.c  |  4 +-
 net/core/lwt_bpf.c|  5 +-
 net/core/neighbour.c  |  8 +--
 net/core/net_namespace.c  |  4 +-
 net/core/rtnetlink.c  | 47 --
 net/dcb/dcbnl.c   | 57 -
 net/decnet/dn_dev.c   |  4 +-
 net/decnet/dn_fib.c   |  6 +-
 net/decnet/dn_route.c |  2 +-
 net/ieee802154/nl802154.c | 29 -
 net/ipv4/devinet.c| 12 ++--
 net/ipv4/fib_frontend.c   |  3 +-
 net/ipv4/ip_tunnel_core.c |  5 +-
 net/ipv4/ipmr.c   |  3 +-
 net/ipv4/route.c  |  3 +-
 net/ipv6/addrconf.c   | 16 +++--
 net/ipv6/addrlabel.c  |  4 +-
 net/ipv6/ila/ila_lwt.c|  3 +-
 net/ipv6/route.c  |  6 +-
 net/ipv6/seg6_iptunnel.c  |  2 +-
 net/mpls/af_mpls.c|  5 +-
 net/mpls/mpls_iptunnel.c  |  2 +-
 net/netfilter/ipset/ip_set_core.c | 27 
 net/netfilter/ipvs/ip_vs_ctl.c| 12 ++--
 net/netfilter/nf_conntrack_netlink.c  | 27 
 net/netfilter/nf_conntrack_proto_dccp.c   |  2 +-
 net/netfilter/nf_conntrack_proto_sctp.c   |  6 +-
 net/netfilter/nf_conntrack_proto_tcp.c|  3 +-
 net/netfilter/nf_nat_core.c   |  5 +-
 net/netfilter/nf_tables_api.c | 27 
 net/netfilter/nfnetlink.c | 11 ++--
 net/netfilter/nfnetlink_acct.c|  3 +-
 net/netfilter/nfnetlink_cthelper.c| 12 ++--
 net/netfilter/nfnetlink_cttimeout.c   |  3 +-
 net/netfilter/nfnetlink_queue.c   |  2 +-
 net/netfilter/nft_compat.c|  2 +-
 net/netlabel/netlabel_cipso_v4.c  | 19 +++---
 net/netlink/genetlink.c   |  2 +-
 net/nfc/netlink.c |  5 +-
 net/openvswitch/datapath.c|  2 +-
 net/openvswitch/flow_netlink.c|  4 +-
 net/openvswitch/vport-vxlan.c |  3 +-
 net/phonet/pn_netlink.c   |  6 +-
 net/qrtr/qrtr.c   |  2 +-
 net/sched/act_api.c   | 20 +++---
 net/sched/act_bpf.c   |  2 +-
 net/sched/act_connmark.c  |  3 +-
 net/sched/act_csum.c  |  2 +-
 net/sched/act_gact.c  |  2 +-
 net/sched/act_ife.c   |  4 +-
 net/sched/act_ipt.c   |  2 +-
 net/sched/act_mirred.c|  2 +-
 net/sched/act_nat.c   |  2 +-
 net/sched/act_pedit.c |  4 +-
 net/sched/act_police.c   

[PATCH v2 1/5] netlink: extended ACK reporting

[Johannes Berg]

From: Johannes Berg 

Add the base infrastructure and UAPI for netlink
extended ACK reporting. All "manual" calls to
netlink_ack() pass NULL for now and thus don't
get extended ACK reporting.

Big thanks goes to Pablo Neira Ayuso for not only
bringing up the whole topic at netconf (again) but
also coming up with the nlattr passing trick and
various other ideads.

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c  |  3 +-
 drivers/infiniband/core/netlink.c |  5 +--
 drivers/scsi/scsi_netlink.c   |  2 +-
 include/linux/netlink.h   | 32 +-
 include/net/netlink.h |  3 +-
 include/uapi/linux/netlink.h  | 24 +
 kernel/audit.c|  2 +-
 net/core/rtnetlink.c  |  3 +-
 net/core/sock_diag.c  |  3 +-
 net/decnet/netfilter/dn_rtmsg.c   |  2 +-
 net/hsr/hsr_netlink.c |  4 +--
 net/netfilter/ipset/ip_set_core.c |  2 +-
 net/netfilter/nfnetlink.c | 22 ++--
 net/netlink/af_netlink.c  | 71 ++-
 net/netlink/af_netlink.h  |  1 +
 net/netlink/genetlink.c   |  3 +-
 net/xfrm/xfrm_user.c  |  3 +-
 17 files changed, 151 insertions(+), 34 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index a90404a0c5ff..4a44830741c1 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -483,7 +483,8 @@ static const struct crypto_link {
[CRYPTO_MSG_DELRNG  - CRYPTO_MSG_BASE] = { .doit = crypto_del_rng },
 };
 
-static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
+static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+  struct netlink_ext_ack *extack)
 {
struct nlattr *attrs[CRYPTOCFGA_MAX+1];
const struct crypto_link *link;
diff --git a/drivers/infiniband/core/netlink.c 
b/drivers/infiniband/core/netlink.c
index 10469b0088b5..b784055423c8 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -146,7 +146,8 @@ int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 EXPORT_SYMBOL(ibnl_put_attr);
 
-static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
+static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+   struct netlink_ext_ack *extack)
 {
struct ibnl_client *client;
int type = nlh->nlmsg_type;
@@ -209,7 +210,7 @@ static void ibnl_rcv_reply_skb(struct sk_buff *skb)
if (nlh->nlmsg_flags & NLM_F_REQUEST)
return;
 
-   ibnl_rcv_msg(skb, nlh);
+   ibnl_rcv_msg(skb, nlh, NULL);
 
msglen = NLMSG_ALIGN(nlh->nlmsg_len);
if (msglen > skb->len)
diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
index 109802f776ed..50e624fb8307 100644
--- a/drivers/scsi/scsi_netlink.c
+++ b/drivers/scsi/scsi_netlink.c
@@ -111,7 +111,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 
 next_msg:
if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
-   netlink_ack(skb, nlh, err);
+   netlink_ack(skb, nlh, err, NULL);
 
skb_pull(skb, rlen);
}
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index da14ab61f363..47562e940e9c 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -62,11 +62,41 @@ netlink_kernel_create(struct net *net, int unit, struct 
netlink_kernel_cfg *cfg)
return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
 }
 
+/**
+ * struct netlink_ext_ack - netlink extended ACK report struct
+ * @_msg: message string to report - don't access directly, use
+ * %NL_SET_ERR_MSG
+ * @bad_attr: attribute with error
+ * @missing_attr: number of missing attr (or 0)
+ * @cookie: cookie data to return to userspace (for success)
+ * @cookie_len: actual cookie data length
+ */
+struct netlink_ext_ack {
+   const char *_msg;
+   const struct nlattr *bad_attr;
+   u16 missing_attr;
+   u8 cookie[NETLINK_MAX_COOKIE_LEN];
+   u8 cookie_len;
+};
+
+/* Always use this macro, this allows later putting the
+ * message into a separate section or such for things
+ * like translation or listing all possible messages.
+ * Currently string formatting is not supported (due
+ * to the lack of an output buffer.)
+ */
+#define NL_SET_ERR_MSG(extack, msg) do {   \
+   static const char *_msg = (msg);\
+   \
+   (extack)->_msg = _msg;  \
+} 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);
 extern void __netlink_clear_multicast_users(struct sock *sk, unsigned int 
group);
-extern void netlink_ack(struct sk_buff 

[PATCH v2 2/5] genetlink: pass extended ACK report down

[Johannes Berg]

From: Johannes Berg 

Pass the extended ACK reporting struct down from
generic netlink to the families, using the existing
struct genl_info for simplicity.

Also add support to set the extended ACK information
from generic netlink users.

Signed-off-by: Johannes Berg 
---
 include/net/genetlink.h | 20 
 net/netlink/genetlink.c |  6 --
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index a34275be3600..b81a4979e1db 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -84,6 +84,7 @@ struct nlattr **genl_family_attrbuf(const struct genl_family 
*family);
  * @attrs: netlink attributes
  * @_net: network namespace
  * @user_ptr: user pointers
+ * @extack: extended ACK report struct
  */
 struct genl_info {
u32 snd_seq;
@@ -94,6 +95,7 @@ struct genl_info {
struct nlattr **attrs;
possible_net_t  _net;
void *  user_ptr[2];
+   struct netlink_ext_ack *extack;
 };
 
 static inline struct net *genl_info_net(struct genl_info *info)
@@ -106,6 +108,24 @@ static inline void genl_info_net_set(struct genl_info 
*info, struct net *net)
write_pnet(>_net, net);
 }
 
+#define GENL_SET_ERR_MSG(info, msg) NL_SET_ERR_MSG((info)->extack, msg)
+
+static inline int genl_err_attr(struct genl_info *info, int err,
+   struct nlattr *attr)
+{
+   info->extack->bad_attr = attr;
+
+   return err;
+}
+
+static inline int genl_err_attr_missing(struct genl_info *info, int err,
+   u16 attr)
+{
+   info->extack->missing_attr = attr;
+
+   return err;
+}
+
 /**
  * struct genl_ops - generic netlink operations
  * @cmd: command identifier
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 57b2e3648bc0..4b598a5999a2 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -497,7 +497,8 @@ static int genl_lock_done(struct netlink_callback *cb)
 
 static int genl_family_rcv_msg(const struct genl_family *family,
   struct sk_buff *skb,
-  struct nlmsghdr *nlh)
+  struct nlmsghdr *nlh,
+  struct netlink_ext_ack *extack)
 {
const struct genl_ops *ops;
struct net *net = sock_net(skb->sk);
@@ -584,6 +585,7 @@ static int genl_family_rcv_msg(const struct genl_family 
*family,
info.genlhdr = nlmsg_data(nlh);
info.userhdr = nlmsg_data(nlh) + GENL_HDRLEN;
info.attrs = attrbuf;
+   info.extack = extack;
genl_info_net_set(, net);
memset(_ptr, 0, sizeof(info.user_ptr));
 
@@ -618,7 +620,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh,
if (!family->parallel_ops)
genl_lock();
 
-   err = genl_family_rcv_msg(family, skb, nlh);
+   err = genl_family_rcv_msg(family, skb, nlh, extack);
 
if (!family->parallel_ops)
genl_unlock();
-- 
2.11.0

[PATCH v2 5/5] netlink: pass extended ACK struct where available

[Johannes Berg]

From: Johannes Berg 

This is an add-on to the previous patch that passes
the extended ACK structure where it's already available
by existing genl_info or extack function arguments.

This was done with this spatch (with some manual
adjustment of indentation):

@@
expression A, B, C, D, E;
identifier fn, info;
@@
fn(..., struct genl_info *info, ...) {
...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, info->extack)
...
}

@@
expression A, B, C, D, E;
identifier fn, info;
@@
fn(..., struct genl_info *info, ...) {
<...
-nla_parse_nested(A, B, C, D, NULL)
+nla_parse_nested(A, B, C, D, info->extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_parse(A, B, C, D, E, NULL)
+nla_parse(A, B, C, D, E, extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, extack)
...
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_parse_nested(A, B, C, D, NULL)
+nla_parse_nested(A, B, C, D, extack)
...>
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nlmsg_validate(A, B, C, D, NULL)
+nlmsg_validate(A, B, C, D, extack)
...>
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_validate(A, B, C, D, NULL)
+nla_validate(A, B, C, D, extack)
...>
}

@@
expression A, B, C;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_validate_nested(A, B, C, NULL)
+nla_validate_nested(A, B, C, extack)
...>
}

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c   |  2 +-
 drivers/net/team/team.c|  3 ++-
 net/ieee802154/nl802154.c  | 10 +-
 net/netfilter/ipvs/ip_vs_ctl.c |  2 +-
 net/netfilter/nfnetlink.c  |  2 +-
 net/netlink/genetlink.c|  2 +-
 net/nfc/netlink.c  |  2 +-
 net/tipc/bearer.c  | 14 +++---
 net/tipc/net.c |  2 +-
 net/tipc/node.c|  8 
 net/wireless/nl80211.c | 33 ++---
 net/xfrm/xfrm_user.c   |  2 +-
 12 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index fc79906c1fe7..b5758768920b 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -523,7 +523,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh,
}
 
err = nlmsg_parse(nlh, crypto_msg_min[type], attrs, CRYPTOCFGA_MAX,
- crypto_policy, NULL);
+ crypto_policy, extack);
if (err < 0)
return err;
 
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 86f227124ba1..65c056e2f705 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2471,7 +2471,8 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, 
struct genl_info *info)
goto team_put;
}
err = nla_parse_nested(opt_attrs, TEAM_ATTR_OPTION_MAX,
-  nl_option, team_nl_option_policy, NULL);
+  nl_option, team_nl_option_policy,
+  info->extack);
if (err)
goto team_put;
if (!opt_attrs[TEAM_ATTR_OPTION_NAME] ||
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index d6b1a1b21909..99f6c254ea77 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -1564,7 +1564,7 @@ static int nl802154_add_llsec_key(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, NL802154_KEY_ATTR_MAX,
 info->attrs[NL802154_ATTR_SEC_KEY],
-nl802154_key_policy, NULL))
+nl802154_key_policy, info->extack))
return -EINVAL;
 
if (!attrs[NL802154_KEY_ATTR_USAGE_FRAMES] ||
@@ -1614,7 +1614,7 @@ static int nl802154_del_llsec_key(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, NL802154_KEY_ATTR_MAX,
 info->attrs[NL802154_ATTR_SEC_KEY],
-nl802154_key_policy, NULL))
+nl802154_key_policy, info->extack))
return -EINVAL;
 
if (ieee802154_llsec_parse_key_id(attrs[NL802154_KEY_ATTR_ID], ) < 0)
@@ -1782,7 +1782,7 @@ static int nl802154_del_llsec_dev(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, 

[PATCH v2 3/5] netlink: allow sending extended ACK with cookie on success

[Johannes Berg]

From: Johannes Berg 

Now that we have extended error reporting and a new message format
for netlink ACK messages, also extend this to be able to return
arbitrary cookie data on success.

This will allow, for example, nl80211 to not send an extra message
for cookies identifying newly created objects, but return those
directly in the ACK message.

The cookie data size is currently limited to 32 bytes (since Jamal
talked about using SHA1 for identifiers.)

Thanks to Jamal Hadi Salim for bringing up this idea during the
discussions.

Signed-off-by: Johannes Berg 
---
 include/linux/netlink.h  |  3 +++
 include/uapi/linux/netlink.h |  4 
 net/netlink/af_netlink.c | 38 --
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 47562e940e9c..2133353b9a30 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -62,6 +62,9 @@ netlink_kernel_create(struct net *net, int unit, struct 
netlink_kernel_cfg *cfg)
return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
 }
 
+/* this can be increased when necessary - don't expose to userland */
+#define NETLINK_MAX_COOKIE_LEN 32
+
 /**
  * struct netlink_ext_ack - netlink extended ACK report struct
  * @_msg: message string to report - don't access directly, use
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index d1564557d645..7892095b1a16 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -118,12 +118,16 @@ struct nlmsgerr {
  * @NLMSGERR_ATTR_OFFS: error offset in the original message (u32)
  * @NLMSGERR_ATTR_ATTR: top-level attribute that caused the error
  * (or is missing, u16)
+ * @NLMSGERR_ATTR_COOKIE: arbitrary subsystem specific cookie to
+ * be used - in the success case - to identify a created
+ * object or operation or similar (binary)
  */
 enum nlmsgerr_attrs {
NLMSGERR_ATTR_UNUSED,
NLMSGERR_ATTR_MSG,
NLMSGERR_ATTR_OFFS,
NLMSGERR_ATTR_ATTR,
+   NLMSGERR_ATTR_COOKIE,
 };
 
 #define NETLINK_ADD_MEMBERSHIP 1
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c74f56a4fcf1..98e55a59c97e 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2311,6 +2311,9 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr 
*nlh, int err,
if (extack->missing_attr)
acksize += nla_total_size(sizeof(u16));
}
+   } else if (nlk->flags & NETLINK_F_EXT_ACK) {
+   if (extack && extack->cookie_len)
+   acksize += nla_total_size(extack->cookie_len);
}
 
skb = nlmsg_new(acksize, GFP_KERNEL);
@@ -2336,20 +2339,27 @@ void netlink_ack(struct sk_buff *in_skb, struct 
nlmsghdr *nlh, int err,
   !(nlk->flags & NETLINK_F_CAP_ACK) ? nlh->nlmsg_len
 : sizeof(*nlh));
 
-   if (err && nlk->flags & NETLINK_F_EXT_ACK && extack) {
-   if (extack->_msg)
-   WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
-  extack->_msg));
-   if (extack->bad_attr &&
-   !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
-(u8 *)extack->bad_attr >= in_skb->data +
-  in_skb->len))
-   WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
-   (u8 *)extack->bad_attr -
-   in_skb->data));
-   if (extack->missing_attr)
-   WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
-   extack->missing_attr));
+   if (nlk->flags & NETLINK_F_EXT_ACK) {
+   if (err && extack) {
+   if (extack->_msg)
+   WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
+  extack->_msg));
+   if (extack->bad_attr &&
+   !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
+(u8 *)extack->bad_attr >= in_skb->data +
+  in_skb->len))
+   WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
+   (u8 *)extack->bad_attr -
+   in_skb->data));
+   if (extack->missing_attr)
+   WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
+   extack->missing_attr));
+   } else if (!err && extack) {
+   if (extack->cookie_len)
+  

[PATCH v2 0/5]

[Johannes Berg]

Changes since v1:
 * credit Pablo and Jamal
 * incorporate suggestion from David Ahern
 * fix compilation in decnet

Re: [PATCH 1/5] netlink: extended ACK reporting

[Jiri Pirko]

Sat, Apr 08, 2017 at 07:48:56PM CEST, johan...@sipsolutions.net wrote:
>From: Johannes Berg 
>
>Add the base infrastructure and UAPI for netlink
>extended ACK reporting. All "manual" calls to
>netlink_ack() pass NULL for now and thus don't
>get extended ACK reporting.

Why so narrow? :)


>
>Signed-off-by: Johannes Berg 
>---

[...]


>diff --git a/include/linux/netlink.h b/include/linux/netlink.h
>index da14ab61f363..47562e940e9c 100644
>--- a/include/linux/netlink.h
>+++ b/include/linux/netlink.h
>@@ -62,11 +62,41 @@ netlink_kernel_create(struct net *net, int unit, struct 
>netlink_kernel_cfg *cfg)
>   return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
> }
> 
>+/**
>+ * struct netlink_ext_ack - netlink extended ACK report struct
>+ * @_msg: message string to report - don't access directly, use
>+ *%NL_SET_ERR_MSG
>+ * @bad_attr: attribute with error
>+ * @missing_attr: number of missing attr (or 0)
>+ * @cookie: cookie data to return to userspace (for success)
>+ * @cookie_len: actual cookie data length
>+ */
>+struct netlink_ext_ack {
>+  const char *_msg;
>+  const struct nlattr *bad_attr;
>+  u16 missing_attr;
>+  u8 cookie[NETLINK_MAX_COOKIE_LEN];
>+  u8 cookie_len;
>+};
>+
>+/* Always use this macro, this allows later putting the
>+ * message into a separate section or such for things
>+ * like translation or listing all possible messages.
>+ * Currently string formatting is not supported (due
>+ * to the lack of an output buffer.)

Please use 80 cols.

[...]


>@@ -2267,21 +2284,37 @@ int __netlink_dump_start(struct sock *ssk, struct 
>sk_buff *skb,
> }
> EXPORT_SYMBOL(__netlink_dump_start);
> 
>-void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err)
>+void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
>+   const struct netlink_ext_ack *extack)
> {
>   struct sk_buff *skb;
>   struct nlmsghdr *rep;
>   struct nlmsgerr *errmsg;
>   size_t payload = sizeof(*errmsg);
>+  size_t acksize = sizeof(*errmsg);
>   struct netlink_sock *nlk = nlk_sk(NETLINK_CB(in_skb).sk);
> 
>   /* Error messages get the original request appened, unless the user
>-   * requests to cap the error message.
>+   * requests to cap the error message, and get extra error data if
>+   * requested.
>*/
>-  if (!(nlk->flags & NETLINK_F_CAP_ACK) && err)
>-  payload += nlmsg_len(nlh);
>+  if (err) {
>+  if (!(nlk->flags & NETLINK_F_CAP_ACK))
>+  payload += nlmsg_len(nlh);
>+  acksize = payload;
>+  if (nlk->flags & NETLINK_F_EXT_ACK) {
>+  if (extack && extack->_msg)
>+  acksize +=
>+  nla_total_size(strlen(extack->_msg) + 
>1);
>+  if (extack && extack->bad_attr)
>+  acksize += nla_total_size(sizeof(u32));
>+  if (extack &&
>+  (extack->missing_attr || extack->bad_attr))

Attr could be 0, right? I know that on the most of the places 0 is
UNSPEC, but I'm pretty sure not everywhere.


>+  acksize += nla_total_size(sizeof(u16));
>+  }
>+  }
> 
>-  skb = nlmsg_new(payload, GFP_KERNEL);
>+  skb = nlmsg_new(acksize, GFP_KERNEL);
>   if (!skb) {
>   struct sock *sk;
> 

[...]

Look very good. Thanks for taking care of this!

Re: [PATCH 1/5] netlink: extended ACK reporting

[David Ahern]

On 4/8/17 1:48 PM, Johannes Berg wrote:
> @@ -2267,21 +2284,37 @@ int __netlink_dump_start(struct sock *ssk, struct 
> sk_buff *skb,
>  }
>  EXPORT_SYMBOL(__netlink_dump_start);
>  
> -void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err)
> +void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
> +  const struct netlink_ext_ack *extack)
>  {
>   struct sk_buff *skb;
>   struct nlmsghdr *rep;
>   struct nlmsgerr *errmsg;
>   size_t payload = sizeof(*errmsg);
> + size_t acksize = sizeof(*errmsg);
>   struct netlink_sock *nlk = nlk_sk(NETLINK_CB(in_skb).sk);
>  
>   /* Error messages get the original request appened, unless the user
> -  * requests to cap the error message.
> +  * requests to cap the error message, and get extra error data if
> +  * requested.
>*/
> - if (!(nlk->flags & NETLINK_F_CAP_ACK) && err)
> - payload += nlmsg_len(nlh);
> + if (err) {
> + if (!(nlk->flags & NETLINK_F_CAP_ACK))
> + payload += nlmsg_len(nlh);
> + acksize = payload;
> + if (nlk->flags & NETLINK_F_EXT_ACK) {
> + if (extack && extack->_msg)
> + acksize +=
> + nla_total_size(strlen(extack->_msg) + 
> 1);
> + if (extack && extack->bad_attr)
> + acksize += nla_total_size(sizeof(u32));
> + if (extack &&
> + (extack->missing_attr || extack->bad_attr))
> + acksize += nla_total_size(sizeof(u16));

If you create a v3, the extack check can by pulled up to the (flags &
NETLINK_F_EXT_ACK) check.


> + }
> + }
>  
> - skb = nlmsg_new(payload, GFP_KERNEL);
> + skb = nlmsg_new(acksize, GFP_KERNEL);
>   if (!skb) {
>   struct sock *sk;
>  
> @@ -2300,14 +2333,35 @@ void netlink_ack(struct sk_buff *in_skb, struct 
> nlmsghdr *nlh, int err)
> NLMSG_ERROR, payload, 0);
>   errmsg = nlmsg_data(rep);
>   errmsg->error = err;
> - memcpy(>msg, nlh, payload > sizeof(*errmsg) ? nlh->nlmsg_len : 
> sizeof(*nlh));
> + memcpy(>msg, nlh,
> +!(nlk->flags & NETLINK_F_CAP_ACK) ? nlh->nlmsg_len
> +  : sizeof(*nlh));
> +
> + if (err && nlk->flags & NETLINK_F_EXT_ACK) {
> + if (extack && extack->_msg)
> + WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
> +extack->_msg));
> + if (extack && extack->bad_attr &&
> + !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
> +  (u8 *)extack->bad_attr >= in_skb->data +
> +in_skb->len))
> + WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
> + (u8 *)extack->bad_attr -
> + in_skb->data));
> + if (extack && extack->missing_attr)
> + WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
> + extack->missing_attr));

same here

Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

[Jan Engelhardt]

On Saturday 2017-04-08 19:21, Arushi Singhal wrote:

>Replace explicit NULL comparison with ! operator to simplify code.

I still wouldn't do this, for the same reason as before. Comparing to 
NULL explicitly more or less gave an extra guarantee that the other 
operand was also a pointer.

Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events

[Roopa Prabhu]

On 4/8/17, 11:13 AM, David Ahern wrote:
> On 4/8/17 2:06 PM, Roopa Prabhu wrote:
>> On 4/7/17, 2:25 PM, David Ahern wrote:
>>> Changing MTU on a link currently causes 3 messages to be sent to userspace:
>>>
>>> [LINK]11: dummy1:  mtu 1490 qdisc noqueue 
>>> state UNKNOWN group default event PRE_CHANGE_MTU
>>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>>
>>> [LINK]11: dummy1:  mtu 1500 qdisc noqueue 
>>> state UNKNOWN group default event CHANGE_MTU
>>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>>
>>> [LINK]11: dummy1:  mtu 1500 qdisc noqueue 
>>> state UNKNOWN group default
>>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>>
>>> Remove the PRE_CHANGE_MTU and CHANGE_MTU messages.
>>>
>>>
>> This change is good... multiple notifications for the same event does not 
>> help in large scale links setups. However, this
>> reverts what vlad was trying to do with his patchset. Vlad's patch-set 
>> relies on the rtnl notifications generated from
>> notifiers (rtnetlink_event) to add  specific event (IFLA_EVENT) in 
>> notifications.
>>
>> The third notification in your example above is the correct one and is an 
>> aggregate notification for a set of changes, but
>> it cannot really fill in all types of events in the single IFLA_EVENT 
>> attribute as it stands today.  IFLA_EVENT should be
>> a bitmask to include all events in this case (i had indicated this in vlads 
>> first version).
>>
> Agreed. I think it would be best to revert def12888c161 before the UAPI
> goes out.
>
> The change can instead add the IFLA_EVENT as a bitmask mentioned here to
> note the changes in a setlink. On top of that, remove the notifications
> for the events I mentioned in this set to reduce the overhead on userspace.

ack

Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events

[David Ahern]

On 4/8/17 2:06 PM, Roopa Prabhu wrote:
> On 4/7/17, 2:25 PM, David Ahern wrote:
>> Changing MTU on a link currently causes 3 messages to be sent to userspace:
>>
>> [LINK]11: dummy1:  mtu 1490 qdisc noqueue state 
>> UNKNOWN group default event PRE_CHANGE_MTU
>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>
>> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
>> UNKNOWN group default event CHANGE_MTU
>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>
>> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
>> UNKNOWN group default
>> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>
>> Remove the PRE_CHANGE_MTU and CHANGE_MTU messages.
>>
>>
> 
> This change is good... multiple notifications for the same event does not 
> help in large scale links setups. However, this
> reverts what vlad was trying to do with his patchset. Vlad's patch-set relies 
> on the rtnl notifications generated from
> notifiers (rtnetlink_event) to add  specific event (IFLA_EVENT) in 
> notifications.
> 
> The third notification in your example above is the correct one and is an 
> aggregate notification for a set of changes, but
> it cannot really fill in all types of events in the single IFLA_EVENT 
> attribute as it stands today.  IFLA_EVENT should be
> a bitmask to include all events in this case (i had indicated this in vlads 
> first version).
> 

Agreed. I think it would be best to revert def12888c161 before the UAPI
goes out.

The change can instead add the IFLA_EVENT as a bitmask mentioned here to
note the changes in a setlink. On top of that, remove the notifications
for the events I mentioned in this set to reduce the overhead on userspace.

Re: [PATCH net-next v2] can: initial support for network namespaces

[Oliver Hartkopp]

Hi Mario,

I started to convert the statistics ... but there's some code adaption 
missing in proc.c


Is this the right way to start?

diff --git a/include/net/netns/can.h b/include/net/netns/can.h
index e8beba772f1a..a8866ae1788f 100644
--- a/include/net/netns/can.h
+++ b/include/net/netns/can.h
@@ -8,6 +8,8 @@
 #include 

 struct dev_rcv_lists;
+struct s_stats;
+struct s_pstats;

 struct netns_can {
 #if IS_ENABLED(CONFIG_PROC_FS)
@@ -26,6 +28,8 @@ struct netns_can {
/* receive filters subscribed for 'all' CAN devices */
struct dev_rcv_lists *can_rx_alldev_list;
spinlock_t can_rcvlists_lock;
+   struct s_stats *can_stats;  /* packet statistics */
+   struct s_pstats *can_pstats;/* receive list statistics */
 };

 #endif /* __NETNS_CAN_H__ */
diff --git a/net/can/af_can.c b/net/can/af_can.c
index abf7d854a94d..db35d6a5ac26 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -84,8 +84,6 @@ static const struct can_proto *proto_tab[CAN_NPROTO] 
__read_mostly;

 static DEFINE_MUTEX(proto_tab_lock);

 struct timer_list can_stattimer;   /* timer for statistics update */
-struct s_statscan_stats;   /* packet statistics */
-struct s_pstats   can_pstats;  /* receive list statistics */

 static atomic_t skbcounter = ATOMIC_INIT(0);

@@ -223,6 +221,7 @@ int can_send(struct sk_buff *skb, int loop)
 {
struct sk_buff *newskb = NULL;
struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
+   struct net *net = dev_net(skb->dev);
int err = -EINVAL;

if (skb->len == CAN_MTU) {
@@ -311,8 +310,8 @@ int can_send(struct sk_buff *skb, int loop)
netif_rx_ni(newskb);

/* update statistics */
-   can_stats.tx_frames++;
-   can_stats.tx_frames_delta++;
+   net->can.can_stats->tx_frames++;
+   net->can.can_stats->tx_frames_delta++;

return 0;

@@ -501,9 +500,9 @@ int can_rx_register(struct net *net, struct 
net_device *dev, canid_t can_id,

hlist_add_head_rcu(>list, rl);
d->entries++;

-   can_pstats.rcv_entries++;
-   if (can_pstats.rcv_entries_max < can_pstats.rcv_entries)
-   can_pstats.rcv_entries_max = can_pstats.rcv_entries;
+   net->can.can_pstats->rcv_entries++;
+		if (net->can.can_pstats->rcv_entries_max < 
net->can.can_pstats->rcv_entries)

+   net->can.can_pstats->rcv_entries_max = 
net->can.can_pstats->rcv_entries;
} else {
kmem_cache_free(rcv_cache, r);
err = -ENODEV;
@@ -591,8 +590,8 @@ void can_rx_unregister(struct net *net, struct 
net_device *dev, canid_t can_id,

hlist_del_rcu(>list);
d->entries--;

-   if (can_pstats.rcv_entries > 0)
-   can_pstats.rcv_entries--;
+   if (net->can.can_pstats->rcv_entries > 0)
+   net->can.can_pstats->rcv_entries--;

/* remove device structure requested by NETDEV_UNREGISTER */
if (d->remove_on_zero_entries && !d->entries) {
@@ -686,11 +685,12 @@ static int can_rcv_filter(struct dev_rcv_lists *d, 
struct sk_buff *skb)

 static void can_receive(struct sk_buff *skb, struct net_device *dev)
 {
struct dev_rcv_lists *d;
+   struct net *net = dev_net(dev);
int matches;

/* update statistics */
-   can_stats.rx_frames++;
-   can_stats.rx_frames_delta++;
+   net->can.can_stats->rx_frames++;
+   net->can.can_stats->rx_frames_delta++;

/* create non-zero unique skb identifier together with *skb */
while (!(can_skb_prv(skb)->skbcnt))
@@ -699,10 +699,10 @@ static void can_receive(struct sk_buff *skb, 
struct net_device *dev)

rcu_read_lock();

/* deliver the packet to sockets listening on all devices */
-   matches = can_rcv_filter(dev_net(dev)->can.can_rx_alldev_list, skb);
+   matches = can_rcv_filter(net->can.can_rx_alldev_list, skb);

/* find receive list for this device */
-   d = find_dev_rcv_lists(dev_net(dev), dev);
+   d = find_dev_rcv_lists(net, dev);
if (d)
matches += can_rcv_filter(d, skb);

@@ -712,8 +712,8 @@ static void can_receive(struct sk_buff *skb, struct 
net_device *dev)

consume_skb(skb);

if (matches > 0) {
-   can_stats.matches++;
-   can_stats.matches_delta++;
+   net->can.can_stats->matches++;
+   net->can.can_stats->matches_delta++;
}
 }

@@ -878,6 +878,9 @@ static int can_pernet_init(struct net *net)
net->can.can_rx_alldev_list =
kzalloc(sizeof(struct dev_rcv_lists), GFP_KERNEL);

+   net->can.can_stats = kzalloc(sizeof(struct s_stats), GFP_KERNEL);
+   net->can.can_pstats = kzalloc(sizeof(struct s_pstats), GFP_KERNEL);
+
if (IS_ENABLED(CONFIG_PROC_FS))
can_init_proc(net);

diff --git a/net/can/af_can.h b/net/can/af_can.h
index f273c9d9b129..10d46bd2e9ea 100644
--- 

Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events

[Roopa Prabhu]

On 4/7/17, 2:25 PM, David Ahern wrote:
> Changing MTU on a link currently causes 3 messages to be sent to userspace:
>
> [LINK]11: dummy1:  mtu 1490 qdisc noqueue state 
> UNKNOWN group default event PRE_CHANGE_MTU
> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>
> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
> UNKNOWN group default event CHANGE_MTU
> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>
> [LINK]11: dummy1:  mtu 1500 qdisc noqueue state 
> UNKNOWN group default
> link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>
> Remove the PRE_CHANGE_MTU and CHANGE_MTU messages.
>
>

This change is good... multiple notifications for the same event does not help 
in large scale links setups. However, this
reverts what vlad was trying to do with his patchset. Vlad's patch-set relies 
on the rtnl notifications generated from
notifiers (rtnetlink_event) to add  specific event (IFLA_EVENT) in 
notifications.

The third notification in your example above is the correct one and is an 
aggregate notification for a set of changes, but
it cannot really fill in all types of events in the single IFLA_EVENT attribute 
as it stands today.  IFLA_EVENT should be
a bitmask to include all events in this case (i had indicated this in vlads 
first version).

Re: [PATCH 06/12] audit: Use timespec64 to represent audit timestamps

[Deepa Dinamani]

> I have no problem merging this patch into audit/next for v4.12, would
> you prefer me to do that so at least this patch is merged?

This would be fine.
But, I think whoever takes the last 2 deletion patches should also take them.
I'm not sure how that part works out.

> It would probably make life a small bit easier for us in the audit
> world too as it would reduce the potential merge conflict.  However,
> that's a relatively small thing to worry about.

-Deepa

[PATCH 5/5] netlink: pass extended ACK struct where available

[Johannes Berg]

From: Johannes Berg 

This is an add-on to the previous patch that passes
the extended ACK structure where it's already available
by existing genl_info or extack function arguments.

This was done with this spatch (with some manual
adjustment of indentation):

@@
expression A, B, C, D, E;
identifier fn, info;
@@
fn(..., struct genl_info *info, ...) {
...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, info->extack)
...
}

@@
expression A, B, C, D, E;
identifier fn, info;
@@
fn(..., struct genl_info *info, ...) {
<...
-nla_parse_nested(A, B, C, D, NULL)
+nla_parse_nested(A, B, C, D, info->extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_parse(A, B, C, D, E, NULL)
+nla_parse(A, B, C, D, E, extack)
...>
}

@@
expression A, B, C, D, E;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
...
-nlmsg_parse(A, B, C, D, E, NULL)
+nlmsg_parse(A, B, C, D, E, extack)
...
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_parse_nested(A, B, C, D, NULL)
+nla_parse_nested(A, B, C, D, extack)
...>
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nlmsg_validate(A, B, C, D, NULL)
+nlmsg_validate(A, B, C, D, extack)
...>
}

@@
expression A, B, C, D;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_validate(A, B, C, D, NULL)
+nla_validate(A, B, C, D, extack)
...>
}

@@
expression A, B, C;
identifier fn, extack;
@@
fn(..., struct netlink_ext_ack *extack, ...) {
<...
-nla_validate_nested(A, B, C, NULL)
+nla_validate_nested(A, B, C, extack)
...>
}

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c   |  2 +-
 drivers/net/team/team.c|  3 ++-
 net/ieee802154/nl802154.c  | 10 +-
 net/netfilter/ipvs/ip_vs_ctl.c |  2 +-
 net/netfilter/nfnetlink.c  |  2 +-
 net/netlink/genetlink.c|  2 +-
 net/nfc/netlink.c  |  2 +-
 net/tipc/bearer.c  | 14 +++---
 net/tipc/net.c |  2 +-
 net/tipc/node.c|  8 
 net/wireless/nl80211.c | 33 ++---
 net/xfrm/xfrm_user.c   |  2 +-
 12 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index fc79906c1fe7..b5758768920b 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -523,7 +523,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh,
}
 
err = nlmsg_parse(nlh, crypto_msg_min[type], attrs, CRYPTOCFGA_MAX,
- crypto_policy, NULL);
+ crypto_policy, extack);
if (err < 0)
return err;
 
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 86f227124ba1..65c056e2f705 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2471,7 +2471,8 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, 
struct genl_info *info)
goto team_put;
}
err = nla_parse_nested(opt_attrs, TEAM_ATTR_OPTION_MAX,
-  nl_option, team_nl_option_policy, NULL);
+  nl_option, team_nl_option_policy,
+  info->extack);
if (err)
goto team_put;
if (!opt_attrs[TEAM_ATTR_OPTION_NAME] ||
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index d6b1a1b21909..99f6c254ea77 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -1564,7 +1564,7 @@ static int nl802154_add_llsec_key(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, NL802154_KEY_ATTR_MAX,
 info->attrs[NL802154_ATTR_SEC_KEY],
-nl802154_key_policy, NULL))
+nl802154_key_policy, info->extack))
return -EINVAL;
 
if (!attrs[NL802154_KEY_ATTR_USAGE_FRAMES] ||
@@ -1614,7 +1614,7 @@ static int nl802154_del_llsec_key(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, NL802154_KEY_ATTR_MAX,
 info->attrs[NL802154_ATTR_SEC_KEY],
-nl802154_key_policy, NULL))
+nl802154_key_policy, info->extack))
return -EINVAL;
 
if (ieee802154_llsec_parse_key_id(attrs[NL802154_KEY_ATTR_ID], ) < 0)
@@ -1782,7 +1782,7 @@ static int nl802154_del_llsec_dev(struct sk_buff *skb, 
struct genl_info *info)
 
if (nla_parse_nested(attrs, 

[PATCH 3/5] netlink: allow sending extended ACK with cookie on success

[Johannes Berg]

From: Johannes Berg 

Now that we have extended error reporting and a new message format
for netlink ACK messages, also extend this to be able to return
arbitrary cookie data on success.

This will allow, for example, nl80211 to not send an extra message
for cookies identifying newly created objects, but return those
directly in the ACK message.

The cookie data size is currently limited to 32 bytes (since Jamal
talked about using SHA1 for identifiers.)

Signed-off-by: Johannes Berg 
---
 include/linux/netlink.h  |  3 +++
 include/uapi/linux/netlink.h |  4 
 net/netlink/af_netlink.c | 38 --
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 47562e940e9c..2133353b9a30 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -62,6 +62,9 @@ netlink_kernel_create(struct net *net, int unit, struct 
netlink_kernel_cfg *cfg)
return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
 }
 
+/* this can be increased when necessary - don't expose to userland */
+#define NETLINK_MAX_COOKIE_LEN 32
+
 /**
  * struct netlink_ext_ack - netlink extended ACK report struct
  * @_msg: message string to report - don't access directly, use
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index d1564557d645..7892095b1a16 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -118,12 +118,16 @@ struct nlmsgerr {
  * @NLMSGERR_ATTR_OFFS: error offset in the original message (u32)
  * @NLMSGERR_ATTR_ATTR: top-level attribute that caused the error
  * (or is missing, u16)
+ * @NLMSGERR_ATTR_COOKIE: arbitrary subsystem specific cookie to
+ * be used - in the success case - to identify a created
+ * object or operation or similar (binary)
  */
 enum nlmsgerr_attrs {
NLMSGERR_ATTR_UNUSED,
NLMSGERR_ATTR_MSG,
NLMSGERR_ATTR_OFFS,
NLMSGERR_ATTR_ATTR,
+   NLMSGERR_ATTR_COOKIE,
 };
 
 #define NETLINK_ADD_MEMBERSHIP 1
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 02cffb0a3904..bc1c9e8dd7ef 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2312,6 +2312,9 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr 
*nlh, int err,
(extack->missing_attr || extack->bad_attr))
acksize += nla_total_size(sizeof(u16));
}
+   } else if (nlk->flags & NETLINK_F_EXT_ACK) {
+   if (extack && extack->cookie_len)
+   acksize += nla_total_size(extack->cookie_len);
}
 
skb = nlmsg_new(acksize, GFP_KERNEL);
@@ -2337,20 +2340,27 @@ void netlink_ack(struct sk_buff *in_skb, struct 
nlmsghdr *nlh, int err,
   !(nlk->flags & NETLINK_F_CAP_ACK) ? nlh->nlmsg_len
 : sizeof(*nlh));
 
-   if (err && nlk->flags & NETLINK_F_EXT_ACK) {
-   if (extack && extack->_msg)
-   WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
-  extack->_msg));
-   if (extack && extack->bad_attr &&
-   !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
-(u8 *)extack->bad_attr >= in_skb->data +
-  in_skb->len))
-   WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
-   (u8 *)extack->bad_attr -
-   in_skb->data));
-   if (extack && extack->missing_attr)
-   WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
-   extack->missing_attr));
+   if (nlk->flags & NETLINK_F_EXT_ACK) {
+   if (err) {
+   if (extack && extack->_msg)
+   WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
+  extack->_msg));
+   if (extack && extack->bad_attr &&
+   !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
+(u8 *)extack->bad_attr >= in_skb->data +
+  in_skb->len))
+   WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
+   (u8 *)extack->bad_attr -
+   in_skb->data));
+   if (extack && extack->missing_attr)
+   WARN_ON(nla_put_u16(skb, NLMSGERR_ATTR_ATTR,
+   extack->missing_attr));
+   } else {
+   if (extack && extack->cookie_len)
+   

[PATCH 1/5] netlink: extended ACK reporting

[Johannes Berg]

From: Johannes Berg 

Add the base infrastructure and UAPI for netlink
extended ACK reporting. All "manual" calls to
netlink_ack() pass NULL for now and thus don't
get extended ACK reporting.

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c  |  3 +-
 drivers/infiniband/core/netlink.c |  5 +--
 drivers/scsi/scsi_netlink.c   |  2 +-
 include/linux/netlink.h   | 32 -
 include/net/netlink.h |  3 +-
 include/uapi/linux/netlink.h  | 24 +
 kernel/audit.c|  2 +-
 net/core/rtnetlink.c  |  3 +-
 net/core/sock_diag.c  |  3 +-
 net/hsr/hsr_netlink.c |  4 +--
 net/netfilter/ipset/ip_set_core.c |  2 +-
 net/netfilter/nfnetlink.c | 22 ++--
 net/netlink/af_netlink.c  | 72 ++-
 net/netlink/af_netlink.h  |  1 +
 net/netlink/genetlink.c   |  3 +-
 net/xfrm/xfrm_user.c  |  3 +-
 16 files changed, 151 insertions(+), 33 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index a90404a0c5ff..4a44830741c1 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -483,7 +483,8 @@ static const struct crypto_link {
[CRYPTO_MSG_DELRNG  - CRYPTO_MSG_BASE] = { .doit = crypto_del_rng },
 };
 
-static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
+static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+  struct netlink_ext_ack *extack)
 {
struct nlattr *attrs[CRYPTOCFGA_MAX+1];
const struct crypto_link *link;
diff --git a/drivers/infiniband/core/netlink.c 
b/drivers/infiniband/core/netlink.c
index 10469b0088b5..b784055423c8 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -146,7 +146,8 @@ int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 EXPORT_SYMBOL(ibnl_put_attr);
 
-static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
+static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+   struct netlink_ext_ack *extack)
 {
struct ibnl_client *client;
int type = nlh->nlmsg_type;
@@ -209,7 +210,7 @@ static void ibnl_rcv_reply_skb(struct sk_buff *skb)
if (nlh->nlmsg_flags & NLM_F_REQUEST)
return;
 
-   ibnl_rcv_msg(skb, nlh);
+   ibnl_rcv_msg(skb, nlh, NULL);
 
msglen = NLMSG_ALIGN(nlh->nlmsg_len);
if (msglen > skb->len)
diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
index 109802f776ed..50e624fb8307 100644
--- a/drivers/scsi/scsi_netlink.c
+++ b/drivers/scsi/scsi_netlink.c
@@ -111,7 +111,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 
 next_msg:
if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
-   netlink_ack(skb, nlh, err);
+   netlink_ack(skb, nlh, err, NULL);
 
skb_pull(skb, rlen);
}
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index da14ab61f363..47562e940e9c 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -62,11 +62,41 @@ netlink_kernel_create(struct net *net, int unit, struct 
netlink_kernel_cfg *cfg)
return __netlink_kernel_create(net, unit, THIS_MODULE, cfg);
 }
 
+/**
+ * struct netlink_ext_ack - netlink extended ACK report struct
+ * @_msg: message string to report - don't access directly, use
+ * %NL_SET_ERR_MSG
+ * @bad_attr: attribute with error
+ * @missing_attr: number of missing attr (or 0)
+ * @cookie: cookie data to return to userspace (for success)
+ * @cookie_len: actual cookie data length
+ */
+struct netlink_ext_ack {
+   const char *_msg;
+   const struct nlattr *bad_attr;
+   u16 missing_attr;
+   u8 cookie[NETLINK_MAX_COOKIE_LEN];
+   u8 cookie_len;
+};
+
+/* Always use this macro, this allows later putting the
+ * message into a separate section or such for things
+ * like translation or listing all possible messages.
+ * Currently string formatting is not supported (due
+ * to the lack of an output buffer.)
+ */
+#define NL_SET_ERR_MSG(extack, msg) do {   \
+   static const char *_msg = (msg);\
+   \
+   (extack)->_msg = _msg;  \
+} 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);
 extern void __netlink_clear_multicast_users(struct sock *sk, unsigned int 
group);
-extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
+extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
+   const struct netlink_ext_ack *extack);
 extern int 

[PATCH 4/5] netlink: pass extended ACK struct to parsing functions

[Johannes Berg]

From: Johannes Berg 

Pass the new extended ACK reporting struct to all of the
generic netlink parsing functions. For now, pass NULL in
almost all callers (except for some in the core.)

Signed-off-by: Johannes Berg 
---
 crypto/crypto_user.c  |  2 +-
 drivers/block/drbd/drbd_nla.c |  2 +-
 drivers/infiniband/core/addr.c|  2 +-
 drivers/infiniband/core/iwpm_util.c   |  6 +-
 drivers/infiniband/core/sa_query.c|  4 +-
 drivers/net/macsec.c  | 10 +--
 drivers/net/team/team.c   |  2 +-
 drivers/net/veth.c|  3 +-
 drivers/net/wireless/ath/ath10k/testmode.c|  4 +-
 drivers/net/wireless/ath/ath6kl/testmode.c|  4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c |  3 +-
 drivers/net/wireless/mac80211_hwsim.c |  4 +-
 drivers/net/wireless/marvell/mwifiex/cfg80211.c   |  4 +-
 drivers/net/wireless/ti/wlcore/testmode.c |  3 +-
 drivers/net/wireless/ti/wlcore/vendor_cmd.c   |  4 +-
 include/net/genetlink.h   |  8 ++-
 include/net/netlink.h | 33 +++---
 include/net/rtnetlink.h   |  3 +-
 lib/nlattr.c  | 28 +---
 net/8021q/vlan_netlink.c  |  3 +-
 net/bridge/br_mdb.c   |  3 +-
 net/bridge/br_netlink.c   |  4 +-
 net/bridge/br_netlink_tunnel.c|  4 +-
 net/can/gw.c  |  2 +-
 net/core/fib_rules.c  |  4 +-
 net/core/lwt_bpf.c|  5 +-
 net/core/neighbour.c  |  8 +--
 net/core/net_namespace.c  |  4 +-
 net/core/rtnetlink.c  | 47 --
 net/dcb/dcbnl.c   | 57 -
 net/decnet/dn_dev.c   |  4 +-
 net/decnet/dn_fib.c   |  6 +-
 net/decnet/dn_route.c |  2 +-
 net/ieee802154/nl802154.c | 29 -
 net/ipv4/devinet.c| 12 ++--
 net/ipv4/fib_frontend.c   |  3 +-
 net/ipv4/ip_tunnel_core.c |  5 +-
 net/ipv4/ipmr.c   |  3 +-
 net/ipv4/route.c  |  3 +-
 net/ipv6/addrconf.c   | 16 +++--
 net/ipv6/addrlabel.c  |  4 +-
 net/ipv6/ila/ila_lwt.c|  3 +-
 net/ipv6/route.c  |  6 +-
 net/ipv6/seg6_iptunnel.c  |  2 +-
 net/mpls/af_mpls.c|  5 +-
 net/mpls/mpls_iptunnel.c  |  2 +-
 net/netfilter/ipset/ip_set_core.c | 27 
 net/netfilter/ipvs/ip_vs_ctl.c| 12 ++--
 net/netfilter/nf_conntrack_netlink.c  | 27 
 net/netfilter/nf_conntrack_proto_dccp.c   |  2 +-
 net/netfilter/nf_conntrack_proto_sctp.c   |  6 +-
 net/netfilter/nf_conntrack_proto_tcp.c|  3 +-
 net/netfilter/nf_nat_core.c   |  5 +-
 net/netfilter/nf_tables_api.c | 27 
 net/netfilter/nfnetlink.c | 11 ++--
 net/netfilter/nfnetlink_acct.c|  3 +-
 net/netfilter/nfnetlink_cthelper.c| 12 ++--
 net/netfilter/nfnetlink_cttimeout.c   |  3 +-
 net/netfilter/nfnetlink_queue.c   |  2 +-
 net/netfilter/nft_compat.c|  2 +-
 net/netlabel/netlabel_cipso_v4.c  | 19 +++---
 net/netlink/genetlink.c   |  2 +-
 net/nfc/netlink.c |  5 +-
 net/openvswitch/datapath.c|  2 +-
 net/openvswitch/flow_netlink.c|  4 +-
 net/openvswitch/vport-vxlan.c |  3 +-
 net/phonet/pn_netlink.c   |  6 +-
 net/qrtr/qrtr.c   |  2 +-
 net/sched/act_api.c   | 20 +++---
 net/sched/act_bpf.c   |  2 +-
 net/sched/act_connmark.c  |  3 +-
 net/sched/act_csum.c  |  2 +-
 net/sched/act_gact.c  |  2 +-
 net/sched/act_ife.c   |  4 +-
 net/sched/act_ipt.c   |  2 +-
 net/sched/act_mirred.c|  2 +-
 net/sched/act_nat.c   |  2 +-
 net/sched/act_pedit.c |  4 +-
 net/sched/act_police.c   

[PATCH 0/5] extended netlink ACK reporting

[Johannes Berg]

Hi,

After testing and fixing the ack message length calculation,
this now works.

The UAPI changes are like before - the ACK message format becomes
 [nlmsg header]
 [ack header]
 [request nlmsg header]
 [request nlmsg body (already optional) - length aligned]
 [extended ACK TLVs - this is NEW]

The extended ACK TLVs currently are:
For the error case:
 * MSG - string message
 * OFFS - offset of problem (e.g. malformed attribute)
  in the request message
 * ATTR - missing attribute ID
For the success case:
 * COOKIE - arbitrary per-subsystem cookie to identify
the newly created object or similar

The whole behaviour can be enabled/disabled/queried using
a new socket option NETLINK_EXT_ACK.

johannes

[PATCH 2/5] genetlink: pass extended ACK report down

[Johannes Berg]

From: Johannes Berg 

Pass the extended ACK reporting struct down from
generic netlink to the families, using the existing
struct genl_info for simplicity.

Also add support to set the extended ACK information
from generic netlink users.

Signed-off-by: Johannes Berg 
---
 include/net/genetlink.h | 20 
 net/netlink/genetlink.c |  6 --
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index a34275be3600..b81a4979e1db 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -84,6 +84,7 @@ struct nlattr **genl_family_attrbuf(const struct genl_family 
*family);
  * @attrs: netlink attributes
  * @_net: network namespace
  * @user_ptr: user pointers
+ * @extack: extended ACK report struct
  */
 struct genl_info {
u32 snd_seq;
@@ -94,6 +95,7 @@ struct genl_info {
struct nlattr **attrs;
possible_net_t  _net;
void *  user_ptr[2];
+   struct netlink_ext_ack *extack;
 };
 
 static inline struct net *genl_info_net(struct genl_info *info)
@@ -106,6 +108,24 @@ static inline void genl_info_net_set(struct genl_info 
*info, struct net *net)
write_pnet(>_net, net);
 }
 
+#define GENL_SET_ERR_MSG(info, msg) NL_SET_ERR_MSG((info)->extack, msg)
+
+static inline int genl_err_attr(struct genl_info *info, int err,
+   struct nlattr *attr)
+{
+   info->extack->bad_attr = attr;
+
+   return err;
+}
+
+static inline int genl_err_attr_missing(struct genl_info *info, int err,
+   u16 attr)
+{
+   info->extack->missing_attr = attr;
+
+   return err;
+}
+
 /**
  * struct genl_ops - generic netlink operations
  * @cmd: command identifier
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 57b2e3648bc0..4b598a5999a2 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -497,7 +497,8 @@ static int genl_lock_done(struct netlink_callback *cb)
 
 static int genl_family_rcv_msg(const struct genl_family *family,
   struct sk_buff *skb,
-  struct nlmsghdr *nlh)
+  struct nlmsghdr *nlh,
+  struct netlink_ext_ack *extack)
 {
const struct genl_ops *ops;
struct net *net = sock_net(skb->sk);
@@ -584,6 +585,7 @@ static int genl_family_rcv_msg(const struct genl_family 
*family,
info.genlhdr = nlmsg_data(nlh);
info.userhdr = nlmsg_data(nlh) + GENL_HDRLEN;
info.attrs = attrbuf;
+   info.extack = extack;
genl_info_net_set(, net);
memset(_ptr, 0, sizeof(info.user_ptr));
 
@@ -618,7 +620,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh,
if (!family->parallel_ops)
genl_lock();
 
-   err = genl_family_rcv_msg(family, skb, nlh);
+   err = genl_family_rcv_msg(family, skb, nlh, extack);
 
if (!family->parallel_ops)
genl_unlock();
-- 
2.11.0

Re: [PATCH net-next v2] can: initial support for network namespaces

[Oliver Hartkopp]

Hello Mario,

unfortunately Marc pushed this patch before I finally was able to review 
it ... :-(


Some questions:

On 02/21/2017 12:19 PM, Mario Kicherer wrote:

This patch adds initial support for network namespaces. The changes only
enable support in the CAN raw, proc and af_can code. GW and BCM still
have their checks that ensure that they are used only from the main
namespace.

The patch boils down to moving the global structures, i.e. the global
filter list and their /proc stats, into a per-namespace structure and passing
around the corresponding "struct net" in a lot of different places.


(..)


diff --git a/include/net/netns/can.h b/include/net/netns/can.h
new file mode 100644
index 000..e8beba7
--- /dev/null
+++ b/include/net/netns/can.h
@@ -0,0 +1,31 @@
+/*
+ * can in net namespaces
+ */
+
+#ifndef __NETNS_CAN_H__
+#define __NETNS_CAN_H__
+
+#include 
+
+struct dev_rcv_lists;
+
+struct netns_can {
+#if IS_ENABLED(CONFIG_PROC_FS)
+   struct proc_dir_entry *proc_dir;
+   struct proc_dir_entry *pde_version;
+   struct proc_dir_entry *pde_stats;
+   struct proc_dir_entry *pde_reset_stats;
+   struct proc_dir_entry *pde_rcvlist_all;
+   struct proc_dir_entry *pde_rcvlist_fil;
+   struct proc_dir_entry *pde_rcvlist_inv;
+   struct proc_dir_entry *pde_rcvlist_sff;
+   struct proc_dir_entry *pde_rcvlist_eff;
+   struct proc_dir_entry *pde_rcvlist_err;
+#endif
+
+   /* receive filters subscribed for 'all' CAN devices */
+   struct dev_rcv_lists *can_rx_alldev_list;
+   spinlock_t can_rcvlists_lock;


You didn't include the statistics here:

+   struct s_stats *can_stats;  /* packet statistics */
+   struct s_pstats *can_pstats;/* receive list statistics */

which need to be per-net too, right?

(..)

@@ -877,6 +871,41 @@ static int can_notifier(struct notifier_block *nb, 
unsigned long msg,
return NOTIFY_DONE;
 }

+int can_pernet_init(struct net *net)
+{
+   net->can.can_rcvlists_lock =
+   __SPIN_LOCK_UNLOCKED(net->can.can_rcvlists_lock);
+   net->can.can_rx_alldev_list =
+   kzalloc(sizeof(struct dev_rcv_lists), GFP_KERNEL);
+   memset(net->can.can_rx_alldev_list, 0, sizeof(struct dev_rcv_lists));
+
+   if (IS_ENABLED(CONFIG_PROC_FS))
+   can_init_proc(net);
+
+   return 0;
+}
+
+void can_pernet_exit(struct net *net)
+{
+   struct net_device *dev;
+
+   if (IS_ENABLED(CONFIG_PROC_FS))
+   can_remove_proc(net);
+
+   /* remove created dev_rcv_lists from still registered CAN devices */
+   rcu_read_lock();
+   for_each_netdev_rcu(net, dev) {
+   if (dev->type == ARPHRD_CAN && dev->ml_priv) {
+   struct dev_rcv_lists *d = dev->ml_priv;
+
+   BUG_ON(d->entries);
+   kfree(d);
+   dev->ml_priv = NULL;
+   }
+   }
+   rcu_read_unlock();


You do a kzalloc(sizeof(struct dev_rcv_lists), GFP_KERNEL) in 
can_pernet_init().


Doesn't it need a kfree(net->can.can_rx_alldev_list) then??

Regards,
Oliver

[PATCH] net: netfilter: Replace explicit NULL comparisons

[Arushi Singhal]

Replace explicit NULL comparison with ! operator to simplify code.

Signed-off-by: Arushi Singhal 
---
 net/netfilter/nf_conntrack_broadcast.c |  2 +-
 net/netfilter/nf_conntrack_core.c  |  2 +-
 net/netfilter/nf_conntrack_ecache.c|  4 +--
 net/netfilter/nf_conntrack_helper.c|  4 +--
 net/netfilter/nf_conntrack_proto.c |  4 +--
 net/netfilter/nf_log.c |  2 +-
 net/netfilter/nf_nat_redirect.c|  2 +-
 net/netfilter/nf_tables_api.c  | 62 +-
 net/netfilter/nfnetlink_log.c  |  6 ++--
 net/netfilter/nfnetlink_queue.c|  8 ++---
 net/netfilter/nft_compat.c |  4 +--
 net/netfilter/nft_ct.c | 10 +++---
 net/netfilter/nft_dynset.c | 14 
 net/netfilter/nft_log.c| 14 
 net/netfilter/nft_lookup.c |  2 +-
 net/netfilter/nft_payload.c|  4 +--
 net/netfilter/nft_set_hash.c   |  4 +--
 net/netfilter/x_tables.c   |  8 ++---
 net/netfilter/xt_TCPMSS.c  |  4 +--
 net/netfilter/xt_addrtype.c|  2 +-
 net/netfilter/xt_connlimit.c   |  2 +-
 net/netfilter/xt_conntrack.c   |  2 +-
 net/netfilter/xt_hashlimit.c   |  4 +--
 net/netfilter/xt_recent.c  |  6 ++--
 24 files changed, 88 insertions(+), 88 deletions(-)

diff --git a/net/netfilter/nf_conntrack_broadcast.c 
b/net/netfilter/nf_conntrack_broadcast.c
index 4e99cca61612..a016d47e5a80 100644
--- a/net/netfilter/nf_conntrack_broadcast.c
+++ b/net/netfilter/nf_conntrack_broadcast.c
@@ -42,7 +42,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
 
rcu_read_lock();
in_dev = __in_dev_get_rcu(rt->dst.dev);
-   if (in_dev != NULL) {
+   if (in_dev) {
for_primary_ifa(in_dev) {
if (ifa->ifa_broadcast == iph->daddr) {
mask = ifa->ifa_mask;
diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index ffb78e5f7b70..282d7ec1acba 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1345,7 +1345,7 @@ nf_conntrack_in(struct net *net, u_int8_t pf, unsigned 
int hooknum,
/* It may be an special packet, error, unclean...
 * inverse of the return code tells to the netfilter
 * core what to do with the packet. */
-   if (l4proto->error != NULL) {
+   if (l4proto->error) {
ret = l4proto->error(net, tmpl, skb, dataoff, pf, hooknum);
if (ret <= 0) {
NF_CT_STAT_INC_ATOMIC(net, error);
diff --git a/net/netfilter/nf_conntrack_ecache.c 
b/net/netfilter/nf_conntrack_ecache.c
index da9df2d56e66..11184cae5329 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -266,7 +266,7 @@ int nf_conntrack_register_notifier(struct net *net,
mutex_lock(_ct_ecache_mutex);
notify = rcu_dereference_protected(net->ct.nf_conntrack_event_cb,
   
lockdep_is_held(_ct_ecache_mutex));
-   if (notify != NULL) {
+   if (notify) {
ret = -EBUSY;
goto out_unlock;
}
@@ -302,7 +302,7 @@ int nf_ct_expect_register_notifier(struct net *net,
mutex_lock(_ct_ecache_mutex);
notify = rcu_dereference_protected(net->ct.nf_expect_event_cb,
   
lockdep_is_held(_ct_ecache_mutex));
-   if (notify != NULL) {
+   if (notify) {
ret = -EBUSY;
goto out_unlock;
}
diff --git a/net/netfilter/nf_conntrack_helper.c 
b/net/netfilter/nf_conntrack_helper.c
index 6dc44d9b4190..fda6348a88e5 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -224,9 +224,9 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct 
nf_conn *tmpl,
if (test_bit(IPS_HELPER_BIT, >status))
return 0;
 
-   if (tmpl != NULL) {
+   if (tmpl) {
help = nfct_help(tmpl);
-   if (help != NULL) {
+   if (help) {
helper = help->helper;
set_bit(IPS_HELPER_BIT, >status);
}
diff --git a/net/netfilter/nf_conntrack_proto.c 
b/net/netfilter/nf_conntrack_proto.c
index 2d6ee1803415..cb1e1593fc82 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -307,7 +307,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
int err = 0;
 
 #ifdef CONFIG_SYSCTL
-   if (pn->ctl_table != NULL) {
+   if (pn->ctl_table) {
err = nf_ct_register_sysctl(net,
>ctl_table_header,
"net/netfilter",
@@ -329,7 +329,7 @@ void nf_ct_l4proto_unregister_sysctl(struct net *net,
  

[PATCH] net: netfilter: ipvs: Replace explicit NULL comparison

[Arushi Singhal]

Replace explicit NULL comparison to simplify code.

Signed-off-by: Arushi Singhal 
---
 net/netfilter/ipvs/ip_vs_ctl.c   | 40 
 net/netfilter/ipvs/ip_vs_proto.c | 22 +++---
 2 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 5aeb0dde6ccc..481dcfa5f2c6 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -436,7 +436,7 @@ ip_vs_service_find(struct netns_ipvs *ipvs, int af, __u32 
fwmark, __u16 protocol
svc = __ip_vs_service_find(ipvs, af, protocol, vaddr, FTPPORT);
}
 
-   if (svc == NULL
+   if (!svc
&& atomic_read(>nullsvc_counter)) {
/*
 * Check if the catch-all port (port zero) exists
@@ -910,7 +910,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct 
ip_vs_dest_user_kern *udest,
}
 
dest = kzalloc(sizeof(struct ip_vs_dest), GFP_KERNEL);
-   if (dest == NULL)
+   if (!dest)
return -ENOMEM;
 
dest->stats.cpustats = alloc_percpu(struct ip_vs_cpu_stats);
@@ -983,7 +983,7 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct 
ip_vs_dest_user_kern *udest)
dest = ip_vs_lookup_dest(svc, udest->af, , dport);
rcu_read_unlock();
 
-   if (dest != NULL) {
+   if (dest) {
IP_VS_DBG(1, "%s(): dest already exists\n", __func__);
return -EEXIST;
}
@@ -994,7 +994,7 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct 
ip_vs_dest_user_kern *udest)
 */
dest = ip_vs_trash_get_dest(svc, udest->af, , dport);
 
-   if (dest != NULL) {
+   if (dest) {
IP_VS_DBG_BUF(3, "Get destination %s:%u from trash, "
  "dest->refcnt=%d, service %u/%s:%u\n",
  IP_VS_DBG_ADDR(udest->af, ), ntohs(dport),
@@ -1047,7 +1047,7 @@ ip_vs_edit_dest(struct ip_vs_service *svc, struct 
ip_vs_dest_user_kern *udest)
dest = ip_vs_lookup_dest(svc, udest->af, , dport);
rcu_read_unlock();
 
-   if (dest == NULL) {
+   if (!dest) {
IP_VS_DBG(1, "%s(): dest doesn't exist\n", __func__);
return -ENOENT;
}
@@ -1129,7 +1129,7 @@ ip_vs_del_dest(struct ip_vs_service *svc, struct 
ip_vs_dest_user_kern *udest)
dest = ip_vs_lookup_dest(svc, udest->af, >addr, dport);
rcu_read_unlock();
 
-   if (dest == NULL) {
+   if (!dest) {
IP_VS_DBG(1, "%s(): destination not found!\n", __func__);
return -ENOENT;
}
@@ -1208,7 +1208,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct 
ip_vs_service_user_kern *u,
 
if (u->pe_name && *u->pe_name) {
pe = ip_vs_pe_getbyname(u->pe_name);
-   if (pe == NULL) {
+   if (!pe) {
pr_info("persistence engine module ip_vs_pe_%s "
"not found\n", u->pe_name);
ret = -ENOENT;
@@ -1228,7 +1228,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct 
ip_vs_service_user_kern *u,
 #endif
 
svc = kzalloc(sizeof(struct ip_vs_service), GFP_KERNEL);
-   if (svc == NULL) {
+   if (!svc) {
IP_VS_DBG(1, "%s(): no memory\n", __func__);
ret = -ENOMEM;
goto out_err;
@@ -1299,7 +1299,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct 
ip_vs_service_user_kern *u,
 
 
  out_err:
-   if (svc != NULL) {
+   if (svc) {
ip_vs_unbind_scheduler(svc, sched);
ip_vs_service_free(svc);
}
@@ -1339,7 +1339,7 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct 
ip_vs_service_user_kern *u)
 
if (u->pe_name && *u->pe_name) {
pe = ip_vs_pe_getbyname(u->pe_name);
-   if (pe == NULL) {
+   if (!pe) {
pr_info("persistence engine module ip_vs_pe_%s "
"not found\n", u->pe_name);
ret = -ENOENT;
@@ -1476,7 +1476,7 @@ static void ip_vs_unlink_service(struct ip_vs_service 
*svc, bool cleanup)
  */
 static int ip_vs_del_service(struct ip_vs_service *svc)
 {
-   if (svc == NULL)
+   if (!svc)
return -EEXIST;
ip_vs_unlink_service(svc, false);
 
@@ -2446,14 +2446,14 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user 
*user, unsigned int len)
rcu_read_unlock();
 
if (cmd != IP_VS_SO_SET_ADD
-   && (svc == NULL || svc->protocol != usvc.protocol)) {
+   && (!svc || svc->protocol != usvc.protocol)) {
ret = -ESRCH;
goto out_unlock;
}
 
switch (cmd) {
case IP_VS_SO_SET_ADD:
-   if (svc != NULL)
+   if (svc)
ret = -EEXIST;
else
  

Re: pull-request: can 2017-04-04,Re: pull-request: can 2017-04-04

[David Miller]

From: Marc Kleine-Budde 
Date: Fri, 7 Apr 2017 11:00:20 +0200

> Sorry. The "tag" some git lost, here it is:
 ...
>   git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git 
> tags/linux-can-fixes-for-4.12-20170404

Thanks, and pulled, but you probably meant to name this 
linux-can-fixes-for-4.11-something...
not 4.12-something...

[PATCH 20/22] staging rtl8723bs: Fix indenting errors and an off-by-one mistake in core/rtw_mlme_ext.c

[Larry Finger]

Smatch lists the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:525 _mgt_dispatcher() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:1595 OnAssocReq() error: buffer 
overflow 'pstapriv->sta_aid' 32 <= 32
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:2391 dump_mgntframe_and_wait() 
warn: inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:2420 
dump_mgntframe_and_wait_ack() warn: inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:4969 process_80211d() error: 
testing array offset 'i' after use.
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:5738 linked_status_chk() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:6459 sitesurvey_cmd_hdl() warn: 
inconsistent indenting

The indenting problems were fixed with white-space changes. The error at
line 1595 was the result of an off-by-one error in a for loop. The error
at line 4969 was not fixed as that code lies inside a block of code that
only is needed for 5G channels. This chip only works at 2.4 GHz.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index e0a3cd64777f..18e78d5198c3 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -522,15 +522,14 @@ static void _mgt_dispatcher(struct adapter *padapter, 
struct mlme_handler *ptabl
u8 bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
u8 *pframe = precv_frame->u.hdr.rx_data;
 
- if (ptable->func) {
-/* receive the frames that ra(a1) is my address or ra(a1) is 
bc address. */
-   if (memcmp(GetAddr1Ptr(pframe), 
myid(>eeprompriv), ETH_ALEN) &&
-   memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
-   return;
-
-   ptable->func(padapter, precv_frame);
-   }
+   if (ptable->func) {
+   /* receive the frames that ra(a1) is my address or ra(a1) is bc 
address. */
+   if (memcmp(GetAddr1Ptr(pframe), myid(>eeprompriv), 
ETH_ALEN) &&
+   memcmp(GetAddr1Ptr(pframe), bc_addr, ETH_ALEN))
+   return;
 
+   ptable->func(padapter, precv_frame);
+   }
 }
 
 void mgt_dispatcher(struct adapter *padapter, union recv_frame *precv_frame)
@@ -1575,7 +1574,7 @@ unsigned int OnAssocReq(struct adapter *padapter, union 
recv_frame *precv_frame)
if (pstat->aid > 0) {
DBG_871X("  old AID %d\n", pstat->aid);
} else {
-   for (pstat->aid = 1; pstat->aid <= NUM_STA; pstat->aid++)
+   for (pstat->aid = 1; pstat->aid < NUM_STA; pstat->aid++)
if (pstapriv->sta_aid[pstat->aid - 1] == NULL)
break;
 
@@ -2388,7 +2387,7 @@ s32 dump_mgntframe_and_wait(struct adapter *padapter, 
struct xmit_frame *pmgntfr
pxmitbuf->sctx = NULL;
spin_unlock_irqrestore(>lock_sctx, irqL);
 
-return ret;
+   return ret;
 }
 
 s32 dump_mgntframe_and_wait_ack(struct adapter *padapter, struct xmit_frame 
*pmgntframe)
@@ -2417,7 +2416,7 @@ s32 dump_mgntframe_and_wait_ack(struct adapter *padapter, 
struct xmit_frame *pmg
mutex_unlock(>ack_tx_mutex);
}
 
-return ret;
+   return ret;
 }
 
 static int update_hidden_ssid(u8 *ies, u32 ies_len, u8 hidden_ssid_mode)
@@ -5735,7 +5734,7 @@ void linked_status_chk(struct adapter *padapter)
#else
rx_chk_limit = 8;
#endif
-   link_count_limit = 7; /*  16 sec */
+   link_count_limit = 7; /*  16 sec */
 
/*  Marked by Kurt 20130715 */
/*  For WiDi 3.5 and latered on, they don't ask WiDi sink to do 
roaming, so we could not check rx limit that strictly. */
@@ -6456,7 +6455,7 @@ u8 sitesurvey_cmd_hdl(struct adapter *padapter, u8 *pbuf)
Switch_DM_Func(padapter, DYNAMIC_FUNC_DISABLE, false);
 
/* config the initial gain under scaning, need to write the BB 
registers */
-   initialgain = 0x1e;
+   initialgain = 0x1e;
 
rtw_hal_set_hwreg(padapter, HW_VAR_INITIAL_GAIN, (u8 
*)());
 
-- 
2.12.0

[PATCH 19/22] staging: rtl8723bs: Fix white-space errors in core/rtw_recv.c

[Larry Finger]

Smart reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_recv.c
drivers/staging/rtl8723bs/core/rtw_recv.c:598 portctrl() warn: inconsistent 
indenting
drivers/staging/rtl8723bs/core/rtw_recv.c:838 sta2sta_data_frame() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_recv.c:1547 validate_recv_frame() warn: 
inconsistent indenting

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_recv.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c 
b/drivers/staging/rtl8723bs/core/rtw_recv.c
index 864538e8299d..6e98a1bd3e77 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -595,7 +595,7 @@ union recv_frame *portctrl(struct adapter *adapter, union 
recv_frame *precv_fram
memcpy(_tmp, ptr, 2);
ether_type = ntohs(be_tmp);
 
- if (ether_type == eapol_type)
+   if (ether_type == eapol_type)
prtnframe = precv_frame;
else {
/* free this frame */
@@ -827,17 +827,14 @@ sint sta2sta_data_frame(
sta_addr = pattrib->src;
 
} else if (check_fwstate(pmlmepriv, WIFI_STATION_STATE) == true) {
-   {
-   /*  For Station mode, sa and bssid should always be 
BSSID, and DA is my mac-address */
-   if (memcmp(pattrib->bssid, pattrib->src, ETH_ALEN)) {
-   RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, 
("bssid != TA under STATION_MODE; drop pkt\n"));
-   ret = _FAIL;
-   goto exit;
-   }
-
-   sta_addr = pattrib->bssid;
+   /*  For Station mode, sa and bssid should always be BSSID, and 
DA is my mac-address */
+   if (memcmp(pattrib->bssid, pattrib->src, ETH_ALEN)) {
+   RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("bssid != 
TA under STATION_MODE; drop pkt\n"));
+   ret = _FAIL;
+   goto exit;
}
 
+   sta_addr = pattrib->bssid;
} else if (check_fwstate(pmlmepriv, WIFI_AP_STATE) == true) {
if (bmcast) {
/*  For AP mode, if DA == MCAST, then BSSID should be 
also MCAST */
@@ -1517,6 +1514,7 @@ sint validate_recv_frame(struct adapter *adapter, union 
recv_frame *precv_frame)
u8 type;
u8 subtype;
sint retval = _SUCCESS;
+   u8 bDumpRxPkt;
 
struct rx_pkt_attrib *pattrib = _frame->u.hdr.attrib;
 
@@ -1544,8 +1542,6 @@ sint validate_recv_frame(struct adapter *adapter, union 
recv_frame *precv_frame)
pattrib->mdata = GetMData(ptr);
pattrib->privacy = GetPrivacy(ptr);
pattrib->order = GetOrder(ptr);
-{
-   u8 bDumpRxPkt;
rtw_hal_get_def_var(adapter, HAL_DEF_DBG_DUMP_RXPKT, &(bDumpRxPkt));
if (bDumpRxPkt == 1) /* dump all rx packets */
dump_rx_packet(ptr);
@@ -1553,7 +1549,6 @@ sint validate_recv_frame(struct adapter *adapter, union 
recv_frame *precv_frame)
dump_rx_packet(ptr);
else if ((bDumpRxPkt == 3) && (type == WIFI_DATA_TYPE))
dump_rx_packet(ptr);
-}
 
switch (type) {
case WIFI_MGT_TYPE: /* mgnt */
-- 
2.12.0

[PATCH 13/22] staging: rtl8723bs: Fix indenting mistake in core/rtw_ap.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_ap.c
drivers/staging/rtl8723bs/core/rtw_ap.c:382 expire_timeout_chk() warn: 
inconsistent indenting

Fixing this requires changing the indentatikon of a long for loop.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_ap.c | 99 -
 1 file changed, 47 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c 
b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 9c71692a3a05..68b750275dff 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -379,70 +379,65 @@ void expire_timeout_chk(struct adapter *padapter)
 
spin_unlock_bh(>asoc_list_lock);
 
-if (chk_alive_num) {
+   if (chk_alive_num) {
+   u8 backup_oper_channel = 0;
+   struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+
+   /* switch to correct channel of current network  before issue 
keep-alive frames */
+   if (rtw_get_oper_ch(padapter) != pmlmeext->cur_channel) {
+   backup_oper_channel = rtw_get_oper_ch(padapter);
+   SelectChannel(padapter, pmlmeext->cur_channel);
+   }
 
-   u8 backup_oper_channel = 0;
-   struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
-   /* switch to correct channel of current network  before issue 
keep-alive frames */
-   if (rtw_get_oper_ch(padapter) != pmlmeext->cur_channel) {
-   backup_oper_channel = rtw_get_oper_ch(padapter);
-   SelectChannel(padapter, pmlmeext->cur_channel);
-   }
+   /* issue null data to check sta alive*/
+   for (i = 0; i < chk_alive_num; i++) {
+   int ret = _FAIL;
 
-   /* issue null data to check sta alive*/
-   for (i = 0; i < chk_alive_num; i++) {
-   int ret = _FAIL;
+   psta = rtw_get_stainfo_by_offset(pstapriv, 
chk_alive_list[i]);
+   if (!(psta->state & _FW_LINKED))
+   continue;
 
-   psta = rtw_get_stainfo_by_offset(pstapriv, chk_alive_list[i]);
-   if (!(psta->state & _FW_LINKED))
-   continue;
+   if (psta->state & WIFI_SLEEP_STATE)
+   ret = issue_nulldata(padapter, psta->hwaddr, 0, 
1, 50);
+   else
+   ret = issue_nulldata(padapter, psta->hwaddr, 0, 
3, 50);
 
-   if (psta->state & WIFI_SLEEP_STATE)
-   ret = issue_nulldata(padapter, psta->hwaddr, 0, 1, 50);
-   else
-   ret = issue_nulldata(padapter, psta->hwaddr, 0, 3, 50);
+   psta->keep_alive_trycnt++;
+   if (ret == _SUCCESS) {
+   DBG_871X(
+   "asoc check, sta(" MAC_FMT ") is 
alive\n",
+   MAC_ARG(psta->hwaddr)
+   );
+   psta->expire_to = pstapriv->expire_to;
+   psta->keep_alive_trycnt = 0;
+   continue;
+   } else if (psta->keep_alive_trycnt <= 3) {
 
-   psta->keep_alive_trycnt++;
-   if (ret == _SUCCESS) {
+   DBG_871X(
+   "ack check for asoc expire, 
keep_alive_trycnt =%d\n",
+   psta->keep_alive_trycnt);
+   psta->expire_to = 1;
+   continue;
+   }
 
-   DBG_871X(
-   "asoc check, sta(" MAC_FMT ") is alive\n",
-   MAC_ARG(psta->hwaddr)
-   );
-   psta->expire_to = pstapriv->expire_to;
psta->keep_alive_trycnt = 0;
-   continue;
-   } else if (psta->keep_alive_trycnt <= 3) {
-
-
DBG_871X(
-   "ack check for asoc expire, keep_alive_trycnt 
=%d\n",
-   psta->keep_alive_trycnt
-   );
-   psta->expire_to = 1;
-   continue;
-   }
-
-   psta->keep_alive_trycnt = 0;
-   DBG_871X(
-   "asoc expire "MAC_FMT", state = 0x%x\n",
-   MAC_ARG(psta->hwaddr),
-   psta->state
-   );
-   spin_lock_bh(>asoc_list_lock);
-   if (list_empty(>asoc_list) == false) {
-   list_del_init(>asoc_list);
-   pstapriv->asoc_list_cnt--;
-   updated = ap_free_sta(padapter, psta, false, 

[PATCH 21/22] staging: rtl8723bs: Fix indenting problems in core/rtw_odm.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_odm.c
drivers/staging/rtl8723bs/core/rtw_odm.c:109 rtw_odm_dbg_comp_msg() warn: if 
statement not indented
drivers/staging/rtl8723bs/core/rtw_odm.c:146 rtw_odm_ability_msg() warn: if 
statement not indented

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_odm.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_odm.c 
b/drivers/staging/rtl8723bs/core/rtw_odm.c
index 5bc573386ad1..3144e8ec2fa2 100644
--- a/drivers/staging/rtl8723bs/core/rtw_odm.c
+++ b/drivers/staging/rtl8723bs/core/rtw_odm.c
@@ -107,8 +107,9 @@ void rtw_odm_dbg_comp_msg(void *sel, struct adapter 
*adapter)
DBG_871X_SEL_NL(sel, "odm.DebugComponents = 0x%016llx\n", dbg_comp);
for (i = 0; i < RTW_ODM_COMP_MAX; i++) {
if (odm_comp_str[i])
-   DBG_871X_SEL_NL(sel, "%cBIT%-2d %s\n",
-   (BIT0 << i) & dbg_comp ? '+' : ' ', i, odm_comp_str[i]);
+   DBG_871X_SEL_NL(sel, "%cBIT%-2d %s\n",
+   (BIT0 << i) & dbg_comp ? '+' : ' ',
+   i, odm_comp_str[i]);
}
 }
 
@@ -144,8 +145,9 @@ void rtw_odm_ability_msg(void *sel, struct adapter *adapter)
DBG_871X_SEL_NL(sel, "odm.SupportAbility = 0x%08x\n", ability);
for (i = 0; i < RTW_ODM_ABILITY_MAX; i++) {
if (odm_ability_str[i])
-   DBG_871X_SEL_NL(sel, "%cBIT%-2d %s\n",
-   (BIT0 << i) & ability ? '+' : ' ', i, 
odm_ability_str[i]);
+   DBG_871X_SEL_NL(sel, "%cBIT%-2d %s\n",
+   (BIT0 << i) & ability ? '+' : ' ', i,
+   odm_ability_str[i]);
}
 }
 
-- 
2.12.0

[PATCH 15/22] staging: rtl8723bs: Fix indenting mistakes in core/rtw_mlme.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_mlme.c
drivers/staging/rtl8723bs/core/rtw_mlme.c:862 rtw_survey_event_callback() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_mlme.c:1102 rtw_free_assoc_resources() warn: 
if statement not indented
drivers/staging/rtl8723bs/core/rtw_mlme.c:1165 rtw_indicate_disconnect() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_mlme.c:2830 rtw_restructure_ht_ie() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_mlme.c:2991 rtw_update_ht_cap() warn: 
inconsistent indenting

All of there are simple white-space errors. A typo is also fixed.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 37 +++
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index c95e0626b522..53755e5b97a6 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -856,7 +856,7 @@ void rtw_survey_event_callback(struct adapter   
*adapter, u8 *pbuf)
 
/*  lock pmlmepriv->lock when you accessing network_q */
if ((check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) == false) {
- if (pnetwork->Ssid.Ssid[0] == 0) {
+   if (pnetwork->Ssid.Ssid[0] == 0) {
pnetwork->Ssid.SsidLength = 0;
}
rtw_add_network(adapter, pnetwork);
@@ -1100,8 +1100,7 @@ void rtw_free_assoc_resources(struct adapter *adapter, 
int lock_scanned_queue)
}
 
if (lock_scanned_queue)
-
-   adapter->securitypriv.key_mask = 0;
+   adapter->securitypriv.key_mask = 0;
 
rtw_reset_rx_info(pdbgpriv);
 }
@@ -1162,7 +1161,7 @@ void rtw_indicate_disconnect(struct adapter *padapter)
/* set ips_deny_time to avoid enter IPS before LPS leave */
rtw_set_ips_deny(padapter, 3000);
 
- _clr_fwstate_(pmlmepriv, _FW_LINKED);
+   _clr_fwstate_(pmlmepriv, _FW_LINKED);
 
rtw_clear_scan_deny(padapter);
}
@@ -2826,8 +2825,8 @@ unsigned int rtw_restructure_ht_ie(struct adapter 
*padapter, u8 *in_ie, u8 *out_
if (stbc_rx_enable)
ht_capie.cap_info |= 
cpu_to_le16(IEEE80211_HT_CAP_RX_STBC_1R);/* RX STBC One spatial stream */
 
-   set_mcs_rate_by_mask(ht_capie.supp_mcs_set, MCS_RATE_1R);
-   break;
+   set_mcs_rate_by_mask(ht_capie.supp_mcs_set, MCS_RATE_1R);
+   break;
 
case RF_2T2R:
case RF_1T2R:
@@ -2984,22 +2983,22 @@ void rtw_update_ht_cap(struct adapter *padapter, u8 
*pie, uint ie_len, u8 channe
 #else /* CONFIG_DISABLE_MCS13TO15 */

set_mcs_rate_by_mask(pmlmeinfo->HT_caps.u.HT_cap_element.MCS_rate, MCS_RATE_2R);
 #endif /* CONFIG_DISABLE_MCS13TO15 */
-   }
+   }
 
-   /* switch to the 40M Hz mode accoring to the AP */
-   /* pmlmeext->cur_bwmode = CHANNEL_WIDTH_40; */
-   switch ((pmlmeinfo->HT_info.infos[0] & 0x3)) {
-   case EXTCHNL_OFFSET_UPPER:
-   pmlmeext->cur_ch_offset = HAL_PRIME_CHNL_OFFSET_LOWER;
-   break;
+   /* switch to the 40M Hz mode according to the AP */
+   /* pmlmeext->cur_bwmode = CHANNEL_WIDTH_40; */
+   switch ((pmlmeinfo->HT_info.infos[0] & 0x3)) {
+   case EXTCHNL_OFFSET_UPPER:
+   pmlmeext->cur_ch_offset = HAL_PRIME_CHNL_OFFSET_LOWER;
+   break;
 
-   case EXTCHNL_OFFSET_LOWER:
-   pmlmeext->cur_ch_offset = HAL_PRIME_CHNL_OFFSET_UPPER;
-   break;
+   case EXTCHNL_OFFSET_LOWER:
+   pmlmeext->cur_ch_offset = HAL_PRIME_CHNL_OFFSET_UPPER;
+   break;
 
-   default:
-   pmlmeext->cur_ch_offset = HAL_PRIME_CHNL_OFFSET_DONT_CARE;
-   break;
+   default:
+   pmlmeext->cur_ch_offset = 
HAL_PRIME_CHNL_OFFSET_DONT_CARE;
+   break;
}
}
 
-- 
2.12.0

[PATCH 17/22] staging: rtl8723bs: Fix indenting problem in core/rtw_sta_mgt.c

[Larry Finger]

Sparse reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_sta_mgt.c
drivers/staging/rtl8723bs/core/rtw_sta_mgt.c:25 _rtw_init_stainfo() warn: 
inconsistent indenting

This problem is fixed with a white-spcae change.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_sta_mgt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c 
b/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c
index d7eee6dd7e3b..cb43ec90a648 100644
--- a/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c
+++ b/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c
@@ -22,7 +22,7 @@ void _rtw_init_stainfo(struct sta_info *psta)
 {
memset((u8 *)psta, 0, sizeof(struct sta_info));
 
-spin_lock_init(>lock);
+   spin_lock_init(>lock);
INIT_LIST_HEAD(>list);
INIT_LIST_HEAD(>hash_list);
/* INIT_LIST_HEAD(>asoc_list); */
-- 
2.12.0

[PATCH 16/22] staging: rtl8723bs: Fix some indenting problems and a potential data overrun

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_wlan_util.c
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:67 cckrates_included() warn: if 
statement not indented
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:81 cckratesonly_included() warn: 
if statement not indented
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:815 rtw_camid_alloc() warn: 
should '1 << (cam_id)' be a 64 bit type?

The first two are fixed with white-space changes. The third is fixed by
restricting cam_id to be less than 32.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c 
b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
index c966241df2ea..f485f541e36d 100644
--- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
@@ -65,8 +65,8 @@ int cckrates_included(unsigned char *rate, int ratelen)
 
for (i = 0; i < ratelen; i++) {
if  rate[i]) & 0x7f) == 2)  || (((rate[i]) & 0x7f) == 4) ||
-  (((rate[i]) & 0x7f) == 11)  || (((rate[i]) & 0x7f) 
== 22))
-   return true;
+(((rate[i]) & 0x7f) == 11)  || (((rate[i]) & 0x7f) == 22))
+   return true;
}
 
return false;
@@ -79,8 +79,8 @@ int cckratesonly_included(unsigned char *rate, int ratelen)
 
for (i = 0; i < ratelen; i++) {
if  rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) &&
-  (((rate[i]) & 0x7f) != 11)  && (((rate[i]) & 0x7f) 
!= 22))
-   return false;
+(((rate[i]) & 0x7f) != 11)  && (((rate[i]) & 0x7f) != 22))
+   return false;
}
 
return true;
@@ -811,7 +811,7 @@ s16 rtw_camid_alloc(struct adapter *adapter, struct 
sta_info *sta, u8 kid)
}
 
 bitmap_handle:
-   if (cam_id >= 0)
+   if (cam_id >= 0 && cam_id < 32)
cam_ctl->bitmap |= BIT(cam_id);
 
spin_unlock_bh(_ctl->lock);
-- 
2.12.0

[PATCH 18/22] staging: rtl8723bs: Fix some white-space errors in core/rtw_security.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_security.c
drivers/staging/rtl8723bs/core/rtw_security.c:266 rtw_wep_encrypt() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:433 rtw_seccalctkipmic() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:749 rtw_tkip_encrypt() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:865 rtw_tkip_decrypt() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:1383 aes_cipher() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:1415 aes_cipher() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:1430 aes_cipher() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:1582 rtw_aes_encrypt() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:1651 aes_decipher() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:1739 aes_decipher() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_security.c:1792 aes_decipher() warn: curly 
braces intended?
drivers/staging/rtl8723bs/core/rtw_security.c:1809 aes_decipher() warn: 
inconsistent indenting

All of the above are fixed with white-space changes. A few unneeded
blank lines are deleted.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 469 +-
 1 file changed, 229 insertions(+), 240 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c 
b/drivers/staging/rtl8723bs/core/rtw_security.c
index 698e11e5d0a9..e832f16997b7 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -262,17 +262,15 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 
*pxmitframe)
arcfour_encrypt(, payload+length, 
crc, 4);
 
} else{
-   length = 
pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
+   length = 
pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
*((__le32 *)crc) = getcrc32(payload, length);
arcfour_init(, wepkey, 3+keylength);
arcfour_encrypt(, payload, payload, 
length);
arcfour_encrypt(, payload+length, 
crc, 4);
 
-   pframe += pxmitpriv->frag_len;
-   pframe = (u8 *)RND4((SIZE_PTR)(pframe));
-
+   pframe += pxmitpriv->frag_len;
+   pframe = (u8 *)RND4((SIZE_PTR)(pframe));
}
-
}
 
WEP_SW_ENC_CNT_INC(psecuritypriv, pattrib->ra);
@@ -429,7 +427,7 @@ void rtw_seccalctkipmic(u8 *key, u8 *header, u8 *data, u32 
data_len, u8 *mic_cod
 
/* Michael MIC pseudo header: DA, SA, 3 x 0, Priority */
if (header[1]&1) {   /* ToDS == 1 */
-   rtw_secmicappend(, [16], 6);  /* DA */
+   rtw_secmicappend(, [16], 6);  /* DA */
if (header[1]&2)  /* From Ds == 1 */
rtw_secmicappend(, [24], 6);
else
@@ -746,9 +744,8 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 
*pxmitframe)
arcfour_encrypt(, payload, 
payload, length);
arcfour_encrypt(, 
payload+length, crc, 4);
 
-   pframe += pxmitpriv->frag_len;
-   pframe = (u8 *)RND4((SIZE_PTR)(pframe));
-
+   pframe += pxmitpriv->frag_len;
+   pframe = (u8 *)RND4((SIZE_PTR)(pframe));
}
}
 
@@ -791,10 +788,8 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 
*precvframe)
 
/* 4 start to decrypt recvframe */
if (prxattrib->encrypt == _TKIP_) {
-
stainfo = rtw_get_stainfo(>stapriv, 
>ta[0]);
if (stainfo != NULL) {
-
if (IS_MCAST(prxattrib->ra)) {
static unsigned long start = 0;
static u32 no_gkey_bc_cnt = 0;
@@ -860,8 +855,9 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 
*precvframe)
*((u32 *)crc) = le32_to_cpu(getcrc32(payload, 
length-4));
 
if (crc[3] != payload[length-1] || crc[2] != 
payload[length-2] || crc[1] != payload[length-3] || crc[0] != 
payload[length-4]) {
-   RT_TRACE(_module_rtl871x_security_c_, _drv_err_, 
("rtw_wep_decrypt:icv error crc[3](%x)!=payload[length-1](%x) || 
crc[2](%x)!=payload[length-2](%x) || crc[1](%x)!=payload[length-3](%x) || 
crc[0](%x)!=payload[length-4](%x)\n",
-   

[PATCH 22/22] staging: rtl8723bs: Fix indenting error in core/rtw_pwrctrl.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:641 LeaveAllPowerSaveModeDirect() 
warn: inconsistent indenting

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c 
b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
index c5dd794dac4b..f708dbf5bfd4 100644
--- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
+++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
@@ -638,7 +638,7 @@ void LeaveAllPowerSaveModeDirect(struct adapter *Adapter)
 
rtw_set_rpwm(Adapter, PS_STATE_S4);
 
-   up(>lock);
+   up(>lock);
 
rtw_lps_ctrl_wk_cmd(pri_padapter, LPS_CTRL_LEAVE, 0);
} else{
-- 
2.12.0

[PATCH 07/22] staging: rtl8723bs: Fix potential usage while NULL error in hal/rtl8723b_hal_init.c

[Larry Finger]

Smatch logs the following:

  CHECK   drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c:518 
rtl8723b_FirmwareDownload() error: we previously assumed 'pFirmware' could be 
null (see line 382)

Fixing this error required a rewrite of the error exits from this routine.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c 
b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index b7f6dc7ce318..d40ad03e99a3 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -377,13 +377,13 @@ s32 rtl8723b_FirmwareDownload(struct adapter *padapter, 
bool  bUsedWoWLANFw)
RT_TRACE(_module_hal_init_c_, _drv_notice_, ("+%s, bUsedWoWLANFw:%d\n", 
__func__, bUsedWoWLANFw));
 #endif
pFirmware = kzalloc(sizeof(struct rt_firmware), GFP_KERNEL);
+   if (!pFirmware)
+   return _FAIL;
pBTFirmware = kzalloc(sizeof(struct rt_firmware), GFP_KERNEL);
-
-   if (!pFirmware || !pBTFirmware) {
-   rtStatus = _FAIL;
-   goto exit;
+   if (!pBTFirmware) {
+   kfree(pFirmware);
+   return _FAIL;
}
-
tmp_ps = rtw_read8(padapter, 0xa3);
tmp_ps &= 0xf8;
tmp_ps |= 0x02;
@@ -441,7 +441,7 @@ s32 rtl8723b_FirmwareDownload(struct adapter *padapter, 
bool  bUsedWoWLANFw)
if (pFirmware->ulFwLength > FW_8723B_SIZE) {
rtStatus = _FAIL;
DBG_871X_LEVEL(_drv_emerg_, "Firmware size:%u exceed %u\n", 
pFirmware->ulFwLength, FW_8723B_SIZE);
-   goto exit;
+   goto release_fw1;
}
 
pFirmwareBuf = pFirmware->szFwBuffer;
@@ -517,6 +517,7 @@ s32 rtl8723b_FirmwareDownload(struct adapter *padapter, 
bool  bUsedWoWLANFw)
 exit:
kfree(pFirmware->szFwBuffer);
kfree(pFirmware);
+release_fw1:
kfree(pBTFirmware);
DBG_871X(" <=== rtl8723b_FirmwareDownload()\n");
return rtStatus;
-- 
2.12.0

[PATCH 10/22] staging: rtl8723bs: Fix indenting problem for hal/hal_com.c

[Larry Finger]

Smatch lists the following:

  CHECK   drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
drivers/staging/rtl8723bs/hal/hal_com_phycfg.c:2090 
Hal_ChannelPlanToRegulation() warn: inconsistent indenting

Fixed by changing the white space.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/hal/hal_com.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c 
b/drivers/staging/rtl8723bs/hal/hal_com.c
index 093978d8f2a4..1880d4140bee 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com.c
@@ -1731,11 +1731,11 @@ void rtw_bb_rf_gain_offset(struct adapter *padapter)
for (i = 0; i < ArrayLen; i += 2) {
v1 = Array[i];
v2 = Array[i+1];
-if (v1 == 
padapter->eeprompriv.EEPROMRFGainVal) {
-   DBG_871X("Offset RF Gain. got 
v1 = 0x%x , v2 = 0x%x\n", v1, v2);
-   target = v2;
-   break;
-}
+   if (v1 == padapter->eeprompriv.EEPROMRFGainVal) 
{
+   DBG_871X("Offset RF Gain. got v1 = 0x%x 
, v2 = 0x%x\n", v1, v2);
+   target = v2;
+   break;
+   }
}
DBG_871X("padapter->eeprompriv.EEPROMRFGainVal = 0x%x , 
Gain offset Target Value = 0x%x\n", padapter->eeprompriv.EEPROMRFGainVal, 
target);
PHY_SetRFReg(padapter, RF_PATH_A, 
REG_RF_BB_GAIN_OFFSET, BIT18|BIT17|BIT16|BIT15, target);
-- 
2.12.0

[PATCH 14/22] staging: rtl8723bs: Fix indenting mistakes in core/rtw_ieee80211.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_ieee80211.c
drivers/staging/rtl8723bs/core/rtw_ieee80211.c:83 rtw_is_cckrates_included() 
warn: if statement not indented
drivers/staging/rtl8723bs/core/rtw_ieee80211.c:98 
rtw_is_cckratesonly_included() warn: if statement not indented

These warnings are fixed with white-space changes.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c 
b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
index af44a8cd26f8..4670e72368e5 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
@@ -81,8 +81,8 @@ uint  rtw_is_cckrates_included(u8 *rate)
 
while (rate[i] !=  0) {
if  rate[i]) & 0x7f) == 2)  || (((rate[i]) & 0x7f) 
== 4) ||
-   (((rate[i]) & 0x7f) == 11)  || (((rate[i]) & 0x7f) == 
22))
-   return true;
+(((rate[i]) & 0x7f) == 11)  || (((rate[i]) & 0x7f) 
== 22))
+   return true;
i++;
}
 
@@ -95,16 +95,13 @@ uintrtw_is_cckratesonly_included(u8 *rate)
 
 
while (rate[i] != 0) {
-   if  rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) 
!= 4) &&
-   (((rate[i]) & 0x7f) != 11)  && (((rate[i]) & 
0x7f) != 22))
-
+   if  rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) &&
+(((rate[i]) & 0x7f) != 11)  && (((rate[i]) & 0x7f) != 22))
return false;
-
-   i++;
+   i++;
}
 
return true;
-
 }
 
 int rtw_check_network_type(unsigned char *rate, int ratelen, int channel)
-- 
2.12.0

[PATCH 09/22] staging: rtl8723bs: Fix indening problem in hal/hal_com_phycfg.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
drivers/staging/rtl8723bs/hal/hal_com_phycfg.c:2090 
Hal_ChannelPlanToRegulation() warn: inconsistent indenting

This warning is fixed with a white-space change.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/hal/hal_com_phycfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c 
b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
index 95fff20e9c4b..566b6f0997da 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
@@ -2087,7 +2087,7 @@ void Hal_ChannelPlanToRegulation(struct adapter *Adapter, 
u16 ChannelPlan)
case RT_CHANNEL_DOMAIN_WORLD_FCC2:
pHalData->Regulation2_4G = TXPWR_LMT_FCC;
pHalData->Regulation5G = TXPWR_LMT_FCC;
-   break;
+   break;
case RT_CHANNEL_DOMAIN_WORLD_FCC3:
pHalData->Regulation2_4G = TXPWR_LMT_FCC;
pHalData->Regulation5G = TXPWR_LMT_FCC;
-- 
2.12.0

[PATCH 05/22] staging: rtl8723bs: Fix indenting mistake in os_dep/mlme_linux.c

[Larry Finger]

Smatch reports the following warning:

  CHECK   drivers/staging/rtl8723bs/os_dep/mlme_linux.c
drivers/staging/rtl8723bs/os_dep/mlme_linux.c:149 rtw_os_indicate_disconnect() 
warn: inconsistent indenting

Again, a simple change in the white space fixes this problem.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/os_dep/mlme_linux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/mlme_linux.c 
b/drivers/staging/rtl8723bs/os_dep/mlme_linux.c
index aa1793389797..46315d1a82f7 100644
--- a/drivers/staging/rtl8723bs/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/mlme_linux.c
@@ -145,8 +145,8 @@ void rtw_os_indicate_disconnect(struct adapter *adapter)
 
rtw_indicate_wx_disassoc_event(adapter);
 
-/* modify for CONFIG_IEEE80211W, none 11w also can use the same 
command */
-rtw_reset_securitypriv_cmd(adapter);
+   /* modify for CONFIG_IEEE80211W, none 11w also can use the same command 
*/
+   rtw_reset_securitypriv_cmd(adapter);
 }
 
 void rtw_report_sec_ie(struct adapter *adapter, u8 authmode, u8 *sec_ie)
-- 
2.12.0

[PATCH 12/22] staging: rtl8723bs: Fix possible usage of NULL pointer in core/rtw_debug.c

[Larry Finger]

Smatch reports the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_debug.c
drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() error: we 
previously assumed 'phead' could be null (see line 453)
drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() warn: 
variable dereferenced before check 'phead' (see line 454)

In the code, there are two successive calls to get_head(). The second
is removed.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_debug.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c 
b/drivers/staging/rtl8723bs/core/rtw_debug.c
index d34747b29309..51cef55d3f76 100644
--- a/drivers/staging/rtl8723bs/core/rtw_debug.c
+++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
@@ -451,7 +451,6 @@ int proc_get_survey_info(struct seq_file *m, void *v)
spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
phead = get_list_head(queue);
plist = phead ? get_next(phead) : NULL;
-   plist = get_next(phead);
if ((!phead) || (!plist)) {
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
return 0;
-- 
2.12.0

[PATCH 06/22] staging: rtl8723bs: Fix various errors in os_dep/ioctl_cfg80211.c

[Larry Finger]

Smatch lists the following:

  CHECK   drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:470 
rtw_cfg80211_ibss_indicate_connect() error: we previously assumed 'scanned' 
could be null (see line 466)
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:942 
rtw_cfg80211_set_encryption() warn: inconsistent indenting
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:955 
rtw_cfg80211_set_encryption() error: buffer overflow 
'psecuritypriv->dot11DefKey' 4 <= 4
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1017 
rtw_cfg80211_set_encryption() error: buffer overflow 
'padapter->securitypriv.dot118021XGrpKey' 5 <= 5
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1216 
cfg80211_rtw_set_default_key() warn: inconsistent indenting
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2498 
rtw_cfg80211_monitor_if_xmit_entry() error: we previously assumed 'skb' could 
be null (see line 2495)
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2850 cfg80211_rtw_start_ap() 
warn: if statement not indented
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2860 cfg80211_rtw_start_ap() 
warn: if statement not indented
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3417 
rtw_cfg80211_preinit_wiphy() warn: inconsistent indenting
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3547 rtw_wdev_alloc() info: 
ignoring unreachable code.

The indenting warnings were fixed by simple white space changes.

The section where 'scanned' could be null required an immediate exit from
the routine at that point. A similar fix was required where 'skb' could be null.

The two buffer overflow errors were caused by off-by-one errors. While
locating these problems, another one was found in os_dep/ioctl_linux.c.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 38 +--
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c|  2 +-
 2 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c 
b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index ce7cca68ff7a..d2c66041a561 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -463,9 +463,10 @@ void rtw_cfg80211_ibss_indicate_connect(struct adapter 
*padapter)
}
else
{
-   if (scanned == NULL)
+   if (scanned == NULL) {
rtw_warn_on(1);
-
+   return;
+   }
if (!memcmp(&(scanned->network.Ssid), 
&(pnetwork->Ssid), sizeof(struct ndis_802_11_ssid))
&& !memcmp(scanned->network.MacAddress, 
pnetwork->MacAddress, sizeof(NDIS_802_11_MAC_ADDRESS))
) {
@@ -906,7 +907,7 @@ static int rtw_cfg80211_set_encryption(struct net_device 
*dev, struct ieee_param
param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff)
{
if (param->u.crypt.idx >= WEP_KEYS
-   && param->u.crypt.idx > BIP_MAX_KEYID
+   || param->u.crypt.idx >= BIP_MAX_KEYID
)
{
ret = -EINVAL;
@@ -927,7 +928,7 @@ static int rtw_cfg80211_set_encryption(struct net_device 
*dev, struct ieee_param
wep_key_idx = param->u.crypt.idx;
wep_key_len = param->u.crypt.key_len;
 
-   if ((wep_key_idx > WEP_KEYS) || (wep_key_len <= 0))
+   if ((wep_key_idx >= WEP_KEYS) || (wep_key_len <= 0))
{
ret = -EINVAL;
goto exit;
@@ -939,7 +940,7 @@ static int rtw_cfg80211_set_encryption(struct net_device 
*dev, struct ieee_param
 
wep_key_len = wep_key_len <= 5 ? 5 : 13;
 
-   psecuritypriv->ndisencryptstatus = Ndis802_11Encryption1Enabled;
+   psecuritypriv->ndisencryptstatus = 
Ndis802_11Encryption1Enabled;
psecuritypriv->dot11PrivacyAlgrthm = _WEP40_;
psecuritypriv->dot118021XGrpPrivacy = _WEP40_;
 
@@ -1213,11 +1214,8 @@ static int cfg80211_rtw_set_default_key(struct wiphy 
*wiphy,
struct adapter *padapter = (struct adapter *)rtw_netdev_priv(ndev);
struct security_priv *psecuritypriv = >securitypriv;
 
-   DBG_871X(FUNC_NDEV_FMT" key_index =%d"
-   ", unicast =%d, multicast =%d"
-   ".\n", FUNC_NDEV_ARG(ndev), key_index
-   , unicast, multicast
-   );
+   DBG_871X(FUNC_NDEV_FMT" key_index =%d, unicast =%d, multicast =%d\n",
+FUNC_NDEV_ARG(ndev), key_index, unicast, multicast);
 
if ((key_index < WEP_KEYS) && ((psecuritypriv->dot11PrivacyAlgrthm == 
_WEP40_) || (psecuritypriv->dot11PrivacyAlgrthm == _WEP104_))) /* set wep 

[PATCH 08/22] staging: rtl8723bs: Fix indenting problems in hal/HalHWImg8723B_BB.c

[Larry Finger]

Smatch lists the following:

  CHECK   drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c
drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:314 
ODM_ReadAndConfig_MP_8723B_AGC_TAB() warn: for statement not indented
drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:583 
ODM_ReadAndConfig_MP_8723B_PHY_REG() warn: for statement not indented
drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:586 
ODM_ReadAndConfig_MP_8723B_PHY_REG() warn: inconsistent indenting

These were all fixed with white-space changes.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c 
b/drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c
index b42f06076f93..51d4219177d3 100644
--- a/drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c
+++ b/drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c
@@ -312,7 +312,7 @@ void ODM_ReadAndConfig_MP_8723B_AGC_TAB(PDM_ODM_T pDM_Odm)
*   Discard the following (offset, data) pairs.
*/
while (v1 < 0x4000 && i < ArrayLen-2)
-   READ_NEXT_PAIR(v1, v2, i);
+   READ_NEXT_PAIR(v1, v2, i);
 
i -= 2; /*  prevent from for-loop += 2 */
} else {
@@ -581,9 +581,9 @@ void ODM_ReadAndConfig_MP_8723B_PHY_REG(PDM_ODM_T pDM_Odm)
*   Discard the following (offset, data) pairs.
*/
while (v1 < 0x4000 && i < ArrayLen-2)
-   READ_NEXT_PAIR(v1, v2, i);
+   READ_NEXT_PAIR(v1, v2, i);
 
-   i -= 2; /*  prevent from for-loop += 2 
*/
+   i -= 2; /*  prevent from for-loop += 2 */
} else { /*  Configure matched pairs and skip to end of 
if-else. */
while (v1 < 0x4000 && i < ArrayLen-2) {
odm_ConfigBB_PHY_8723B(pDM_Odm, v1, 
bMaskDWord, v2);
-- 
2.12.0

[PATCH 11/22] staging: rtl8723bs: Fix indenting problems in core/rtw_xmit.c

[Larry Finger]

Smatch logs the following:

  CHECK   drivers/staging/rtl8723bs/core/rtw_xmit.c
drivers/staging/rtl8723bs/core/rtw_xmit.c:277 _rtw_init_xmit_priv() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_xmit.c:294 _rtw_free_xmit_priv() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_xmit.c:295 _rtw_free_xmit_priv() warn: 
inconsistent indenting
drivers/staging/rtl8723bs/core/rtw_xmit.c:946 xmitframe_addmic() warn: 
inconsistent indenting

These are fixed with white-space changes.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/core/rtw_xmit.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c 
b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index 60585540069a..8f2c9a6658bf 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -274,7 +274,7 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct 
adapter *padapter)
rtw_alloc_hwxmits(padapter);
rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
 
-  for (i = 0; i < 4; i++) {
+   for (i = 0; i < 4; i++) {
pxmitpriv->wmm_para_seq[i] = i;
}
 
@@ -290,8 +290,8 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct 
adapter *padapter)
 
 void _rtw_free_xmit_priv(struct xmit_priv *pxmitpriv)
 {
-   int i;
-  struct adapter *padapter = pxmitpriv->adapter;
+   int i;
+   struct adapter *padapter = pxmitpriv->adapter;
struct xmit_frame   *pxmitframe = (struct xmit_frame *) 
pxmitpriv->pxmit_frame_buf;
struct xmit_buf *pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
 
@@ -942,8 +942,8 @@ static s32 xmitframe_addmic(struct adapter *padapter, 
struct xmit_frame *pxmitfr
 
}
 
-  /* if (pqospriv->qos_option == 1) */
-  if (pattrib->qos_en)
+   /* if (pqospriv->qos_option == 1) */
+   if (pattrib->qos_en)
priority[0] = (u8)pxmitframe->attrib.priority;
 
 
-- 
2.12.0

[PATCH 01/22] staging: rtl8723bs: Fix indenting warning in os_dep/xmit_linux.c

[Larry Finger]

Smatch issues the warning

  CHECK   drivers/staging/rtl8723bs/os_dep/xmit_linux.c
drivers/staging/rtl8723bs/os_dep/xmit_linux.c:42 _rtw_pktfile_read() warn: 
inconsistent indenting

A simple indent changes fixes this.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c 
b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
index 5ede3b6fbdec..66dfec18f770 100644
--- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
@@ -39,7 +39,7 @@ uint _rtw_pktfile_read (struct pkt_file *pfile, u8 *rmem, 
uint rlen)
len =  rtw_remainder_len(pfile);
len = (rlen > len)? len: rlen;
 
-   if (rmem)
+   if (rmem)
skb_copy_bits(pfile->pkt, pfile->buf_len-pfile->pkt_len, rmem, 
len);
 
pfile->cur_addr += len;
-- 
2.12.0

[PATCH 02/22] staging: rtl8723bs: Fix indenting warning in os_dep/rtw_proc.c

[Larry Finger]

Smatch lists the following warning:

  CHECK   drivers/staging/rtl8723bs/os_dep/rtw_proc.c
drivers/staging/rtl8723bs/os_dep/rtw_proc.c:102 rtw_drv_proc_open() warn: 
inconsistent indenting

This warning is fixed with a simple change in the white space.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/os_dep/rtw_proc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/rtw_proc.c 
b/drivers/staging/rtl8723bs/os_dep/rtw_proc.c
index 4088381dff35..a2011e06d719 100644
--- a/drivers/staging/rtl8723bs/os_dep/rtw_proc.c
+++ b/drivers/staging/rtl8723bs/os_dep/rtw_proc.c
@@ -99,7 +99,8 @@ static int rtw_drv_proc_open(struct inode *inode, struct file 
*file)
/* struct net_device *dev = proc_get_parent_data(inode); */
ssize_t index = (ssize_t)PDE_DATA(inode);
const struct rtw_proc_hdl *hdl = drv_proc_hdls+index;
-   return single_open(file, hdl->show, NULL);
+
+   return single_open(file, hdl->show, NULL);
 }
 
 static ssize_t rtw_drv_proc_write(struct file *file, const char __user 
*buffer, size_t count, loff_t *pos)
-- 
2.12.0

[PATCH 03/22] staging: rtl8723bs: Fix dereference before check warning in os_dep/recv_linux.c

[Larry Finger]

Smatch lists the following warning:

  CHECK   drivers/staging/rtl8723bs/os_dep/recv_linux.c
drivers/staging/rtl8723bs/os_dep/recv_linux.c:353 rtw_recv_indicatepkt() warn: 
variable dereferenced before check 'precv_frame' (see line 312)

This warning is fixed by removing the test at line 353.

Signed-off-by: Larry Finger 
---
 drivers/staging/rtl8723bs/os_dep/recv_linux.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c 
b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
index c677a5216b54..e731ab4e2bd7 100644
--- a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
@@ -350,8 +350,7 @@ int rtw_recv_indicatepkt(struct adapter *padapter, union 
recv_frame *precv_frame
 _recv_indicatepkt_drop:
 
 /* enqueue back to free_recv_queue */
-if (precv_frame)
-rtw_free_recvframe(precv_frame, pfree_recv_queue);
+rtw_free_recvframe(precv_frame, pfree_recv_queue);
 
 DBG_COUNTER(padapter->rx_logs.os_indicate_err);
 return _FAIL;
-- 
2.12.0