* Re: [patch net-next repost 1/3] idr: Use unsigned long instead of int
From: Eric Dumazet @ 2017-08-16 10:37 UTC (permalink / raw)
To: Chris Mi; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem, mawilcox
In-Reply-To: <1502871244-19870-2-git-send-email-chrism@mellanox.com>
On Wed, 2017-08-16 at 04:14 -0400, Chris Mi wrote:
> IDR uses internally radix tree which uses unsigned long. It doesn't
> makes sense to have index as signed value.
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> block/bsg.c | 8 ++--
> block/genhd.c | 12 +++---
> drivers/atm/nicstar.c | 11 ++---
> drivers/block/drbd/drbd_main.c | 31 ++++++++------
> drivers/block/drbd/drbd_nl.c | 22 +++++-----
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 ++++---
> drivers/block/drbd/drbd_state.c | 34 ++++++++-------
> drivers/block/drbd/drbd_worker.c | 6 +--
> drivers/block/loop.c | 17 +++++---
> drivers/block/nbd.c | 20 +++++----
> drivers/block/zram/zram_drv.c | 9 ++--
> drivers/char/tpm/tpm-chip.c | 10 +++--
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 ++--
> drivers/firewire/core-cdev.c | 18 ++++----
> drivers/firewire/core-device.c | 15 ++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +--
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 ++--
> drivers/gpu/drm/drm_connector.c | 10 +++--
> drivers/gpu/drm/drm_context.c | 20 +++++----
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 ++---
> drivers/gpu/drm/drm_drv.c | 6 ++-
> drivers/gpu/drm/drm_gem.c | 19 +++++----
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +++--
> drivers/gpu/drm/drm_syncobj.c | 18 +++++---
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++++++-----
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 ++--
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +--
> drivers/gpu/drm/i915/i915_gem_context.c | 9 ++--
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 ++--
> drivers/gpu/drm/qxl/qxl_release.c | 14 +++---
> drivers/gpu/drm/sis/sis_mm.c | 8 ++--
> drivers/gpu/drm/tegra/drm.c | 10 +++--
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +++---
> drivers/gpu/drm/via/via_mm.c | 8 ++--
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 ++-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 ++-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 ++--
> drivers/i2c/i2c-core-base.c | 19 +++++----
> drivers/infiniband/core/cm.c | 8 ++--
> drivers/infiniband/core/cma.c | 12 +++---
> drivers/infiniband/core/rdma_core.c | 9 ++--
> drivers/infiniband/core/sa_query.c | 23 +++++-----
> drivers/infiniband/core/ucm.c | 7 ++-
> drivers/infiniband/core/ucma.c | 14 ++++--
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 ++++----
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 ++--
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +--
> drivers/infiniband/hw/mlx4/cm.c | 13 +++---
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 ++-
> drivers/infiniband/hw/qib/qib_init.c | 9 ++--
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 ++---
> drivers/iommu/intel-svm.c | 9 ++--
> drivers/md/dm.c | 13 +++---
> drivers/memstick/core/memstick.c | 10 +++--
> drivers/memstick/core/ms_block.c | 9 ++--
> drivers/memstick/core/mspro_block.c | 12 ++++--
> drivers/mfd/rtsx_pcr.c | 9 ++--
> drivers/misc/c2port/core.c | 7 +--
> drivers/misc/cxl/context.c | 8 ++--
> drivers/misc/cxl/main.c | 15 ++++---
> drivers/misc/mei/main.c | 8 ++--
> drivers/misc/mic/scif/scif_api.c | 11 ++---
> drivers/misc/mic/scif/scif_ports.c | 18 ++++----
> drivers/misc/tifm_core.c | 9 ++--
> drivers/mtd/mtdcore.c | 9 ++--
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 ++-
> drivers/net/ppp/ppp_generic.c | 27 ++++++------
> drivers/net/tap.c | 10 +++--
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++++++----
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +++---
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +++----
> drivers/of/unittest.c | 25 ++++++-----
> drivers/power/supply/bq2415x_charger.c | 16 +++----
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 ++++---
> drivers/power/supply/ds2782_battery.c | 9 ++--
> drivers/powercap/powercap_sys.c | 8 ++--
> drivers/pps/pps.c | 10 +++--
> drivers/rapidio/rio_cm.c | 17 ++++----
> drivers/remoteproc/remoteproc_core.c | 8 ++--
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 ++--
> drivers/scsi/bfa/bfad_im.c | 8 ++--
> drivers/scsi/ch.c | 8 ++--
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +++--
> drivers/scsi/lpfc/lpfc_vport.c | 8 ++--
> drivers/scsi/sg.c | 10 +++--
> drivers/scsi/st.c | 8 ++--
> drivers/staging/greybus/uart.c | 22 +++++-----
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +--
> drivers/target/iscsi/iscsi_target.c | 7 +--
> drivers/target/iscsi/iscsi_target_login.c | 9 ++--
> drivers/target/target_core_device.c | 9 ++--
> drivers/target/target_core_user.c | 13 +++---
> drivers/tee/tee_shm.c | 8 ++--
> drivers/uio/uio.c | 9 ++--
> drivers/usb/class/cdc-acm.c | 24 ++++++-----
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +--
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 ++---
> drivers/vfio/vfio.c | 15 ++++---
> fs/dlm/lock.c | 9 ++--
> fs/dlm/lockspace.c | 6 +--
> fs/dlm/recover.c | 10 ++---
> fs/nfs/nfs4client.c | 9 ++--
> fs/nfsd/nfs4state.c | 8 ++--
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 ++--
> fs/ocfs2/cluster/tcp.c | 10 +++--
> include/linux/idr.h | 26 +++++------
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 ++++----
> kernel/bpf/syscall.c | 20 +++++----
> kernel/cgroup/cgroup.c | 57 +++++++++++++++----------
> kernel/events/core.c | 10 ++---
> kernel/workqueue.c | 15 ++++---
> lib/idr.c | 38 ++++++++++-------
> lib/radix-tree.c | 5 ++-
> mm/memcontrol.c | 11 +++--
> net/9p/client.c | 17 ++++----
> net/9p/util.c | 14 +++---
> net/core/net_namespace.c | 23 +++++-----
> net/mac80211/cfg.c | 23 +++++-----
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +--
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 ++++----
> net/qrtr/qrtr.c | 21 +++++----
> net/rxrpc/conn_client.c | 15 ++++---
> net/sctp/associola.c | 8 ++--
> net/tipc/server.c | 7 +--
> 153 files changed, 956 insertions(+), 736 deletions(-)
This is insane
Have you considered switching the pathological hash table in net/sched
into lib/rhashtable.c ?
Patch would be limited to networking tree.
^ permalink raw reply
* Re: Something hitting my total number of connections to the server
From: Eric Dumazet @ 2017-08-16 10:34 UTC (permalink / raw)
To: Akshat Kakkar; +Cc: netdev
In-Reply-To: <CAA5aLPgekZJgsZV-MAD+F+ggUugQkz12D9Ef=kbP9Ev=Q1QsDw@mail.gmail.com>
On Wed, 2017-08-16 at 10:18 +0530, Akshat Kakkar wrote:
> On Mon, Aug 14, 2017 at 2:37 PM, Akshat Kakkar <akshat.1984@gmail.com> wrote:
> > I have centos 7.3 (Kernel 3.10) running on a server with 128GB RAM and
> > 2 x 10 Core Xeon Processor.
> > I have hosted a webserver on it and enabled ssh for remote maintenance.
> > Previously it was running on Centos 6.3.
> > After upgrading to CentOS 7.3, occasionally (probably when number of
> > hits are more on the server), I am not able to create new connections
> > (neither on web nor on ssh). Existing connections keeps on running
> > fine.
> >
> > I did packet capturing using tcpdump to understand if its some
> > intermediate network issue.
> > What I found was the server is not replying for new SYN requests.
> >
> > So it's clear that its not at all application issue. Also, there are
> > no logs in applications logs for any connections dropped, if any.
> >
> > I check my firewall rules if there is some rate limiting imposed.
> > There is nothing in there.
> >
> > I check tc, if by mistake some rate limiting is imposed. There is
> > nothing in there too.
> >
> > I have increased noOfFiles to 1000000 and other sysctl parameters, but
> > the issue is still there.
> >
> > Has anybody experienced the same?
> >
> > How to go about? Anybody ... Please Help!!!
>
> Its getting lonely out here. Anybody there ???
We wont help you unless you use a recent kernel.
3.10 misses all recent improvements in TCP stack (4 years of hard work)
^ permalink raw reply
* Re: [PATCH net-next V2 1/3] tap: use build_skb() for small packet
From: Eric Dumazet @ 2017-08-16 10:24 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, mst, netdev, linux-kernel, kubakici
In-Reply-To: <2ae570ab-01da-2ea2-0549-3e310158b817@redhat.com>
On Wed, 2017-08-16 at 11:55 +0800, Jason Wang wrote:
>
> On 2017年08月16日 11:45, Eric Dumazet wrote:
> >
> > You do realize that tun_build_skb() is not thread safe ?
>
> Ok, I think the issue if skb_page_frag_refill(), need a spinlock
> probably. Will prepare a patch.
But since tun is used from process context, why don't you use the
per-thread generator (no lock involved)
tcp_sendmsg() uses this for GFP_KERNEL allocations.
Untested patch :
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5892284eb8d05b0678d820bad3d0d2c61a879aeb..c38cd840cc0b7fecf182b23976e36f709cacca1f 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -175,7 +175,6 @@ struct tun_file {
struct list_head next;
struct tun_struct *detached;
struct skb_array tx_array;
- struct page_frag alloc_frag;
};
struct tun_flow_entry {
@@ -578,8 +577,6 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
}
if (tun)
skb_array_cleanup(&tfile->tx_array);
- if (tfile->alloc_frag.page)
- put_page(tfile->alloc_frag.page);
sock_put(&tfile->sk);
}
}
@@ -1272,7 +1269,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
struct virtio_net_hdr *hdr,
int len, int *generic_xdp)
{
- struct page_frag *alloc_frag = &tfile->alloc_frag;
+ struct page_frag *alloc_frag = ¤t->task_frag;
struct sk_buff *skb;
struct bpf_prog *xdp_prog;
int buflen = SKB_DATA_ALIGN(len + TUN_RX_PAD) +
@@ -2580,8 +2577,6 @@ static int tun_chr_open(struct inode *inode, struct file * file)
tfile->sk.sk_write_space = tun_sock_write_space;
tfile->sk.sk_sndbuf = INT_MAX;
- tfile->alloc_frag.page = NULL;
-
file->private_data = tfile;
INIT_LIST_HEAD(&tfile->next);
^ permalink raw reply related
* Re: [PATCH 1/2] mpls: add handlers
From: Amine Kherbouche @ 2017-08-16 10:24 UTC (permalink / raw)
To: Roopa Prabhu, David Lamparter; +Cc: netdev@vger.kernel.org
In-Reply-To: <CAJieiUhp57grbrzNcdyJj+_iKp=rusLfrJjsRPfebAUFEqy89A@mail.gmail.com>
On 08/16/2017 07:30 AM, Roopa Prabhu wrote:
> On Tue, Aug 15, 2017 at 2:37 AM, David Lamparter <equinox@diac24.net> wrote:
>
> [snip]
>
>> I think the reverse is the better option, removing the vpls device
>> information and just going with the route table. My approach to this
>> would be to add a new netlink route attribute "RTA_VPLS" which
>> identifies the vpls device, is stored in the route table, and provides
>> the device ptr needed here.
>> (The control word config should also be on the route.)
>>
>> My reason for thinking this is that the VPLS code needs exactly the same
>> information as does a normal MPLS route: it attaches to an incoming
>> label (decapsulating packets instead of forwarding them), and for TX it
>> does the same operation of looking up a nexthop (possibly with ECMP
>> support) and adding a label stack. The code should, in fact, probably
>> reuse the TX path.
>>
>> This also fits both an 1:1 and 1:n model pretty well. Creating a VPLS
>> head-end netdevice doesn't even need any config. It'd just work like:
>> - ip link add name vpls123 type vpls
>> - ip -f mpls route add \
>> 1234 \ # incoming label for decap
>> vpls vpls123 \ # new attr: VPLS device
>> as 2345 via inet 10.0.0.1 dev eth0 # outgoing label for encap
>>
>> For a 1:n model, one would simply add multiple routes on the same vpls
>> device.
>>
>
> this is a nice model too. But, i don't see how vlans and mac based
> learning will fit in here.
>
> modeling it same as how vxlan l2 overlay tunnels are done seems like a
> great fit.
> The vpls driver can learn mac's per pw tunnel labels. And this l2 fdb
> table per vpls device can also carry dst information similar to how
> vxlan driver does today.
>
I think this is a good idea too, I'll implement this concept in mpls and
have a look at the way vxlan is done to be able to support the l2 part
in vpls driver.
Thanks
^ permalink raw reply
* Re: 100% CPU load when generating traffic to destination network that nexthop is not reachable
From: Paweł Staszewski @ 2017-08-16 10:07 UTC (permalink / raw)
To: Julian Anastasov, Eric Dumazet; +Cc: Linux Kernel Network Developers
In-Reply-To: <alpine.LFD.2.20.1708161026280.1778@ja.home.ssi.bg>
Hi
Patch applied - but no big change - from 0.7Mpps per vlan to 1.2Mpps per
vlan
previously(without patch) 100% cpu load:
bwm-ng v0.6.1 (probing every 0.500s), press 'h' for help
input: /proc/net/dev type: rate
| iface Rx Tx Total
==============================================================================
vlan1002: 0.00 P/s 1.99 P/s
1.99 P/s
vlan1001: 0.00 P/s 717227.12 P/s 717227.12 P/s
enp175s0f0: 2713679.25 P/s 0.00 P/s 2713679.25 P/s
vlan1000: 0.00 P/s 716145.44 P/s 716145.44 P/s
------------------------------------------------------------------------------
total: 2713679.25 P/s 1433374.50 P/s 4147054.00 P/s
With patch (100% cpu load a little better pps performance)
bwm-ng v0.6.1 (probing every 1.000s), press 'h' for help
input: /proc/net/dev type: rate
| iface Rx Tx Total
==============================================================================
vlan1002: 0.00 P/s 1.00 P/s
1.00 P/s
vlan1001: 0.00 P/s 1202161.50 P/s 1202161.50 P/s
enp175s0f0: 3699864.50 P/s 0.00 P/s 3699864.50 P/s
vlan1000: 0.00 P/s 1196870.38 P/s 1196870.38 P/s
------------------------------------------------------------------------------
total: 3699864.50 P/s 2399033.00 P/s 6098897.50 P/s
perf top attached below:
1.90% 0.00% ksoftirqd/39 [kernel.vmlinux] [k] run_ksoftirqd
|
--1.90%--run_ksoftirqd
|
--1.90%--__softirqentry_text_start
|
--1.90%--net_rx_action
|
--1.90%--mlx5e_napi_poll
|
--1.89%--mlx5e_poll_rx_cq
|
--1.88%--mlx5e_handle_rx_cqe
|
--1.85%--napi_gro_receive
|
--1.85%--netif_receive_skb_internal
|
--1.85%--__netif_receive_skb
|
--1.85%--__netif_receive_skb_core
|
--1.85%--ip_rcv
|
--1.85%--ip_rcv_finish
|
--1.83%--ip_forward
|
--1.82%--ip_forward_finish
|
--1.82%--ip_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.77%--neigh_event_send
|
--1.77%--__neigh_event_send
|
--1.74%--_raw_write_lock_bh
|
--1.74%--queued_write_lock
queued_write_lock_slowpath
|
--1.70%--queued_spin_lock_slowpath
1.90% 0.00% ksoftirqd/34 [kernel.vmlinux] [k]
__softirqentry_text_start
|
---__softirqentry_text_start
|
--1.90%--net_rx_action
|
--1.90%--mlx5e_napi_poll
|
--1.89%--mlx5e_poll_rx_cq
|
--1.88%--mlx5e_handle_rx_cqe
|
--1.86%--napi_gro_receive
|
--1.85%--netif_receive_skb_internal
|
--1.85%--__netif_receive_skb
|
--1.85%--__netif_receive_skb_core
|
--1.85%--ip_rcv
|
--1.85%--ip_rcv_finish
|
--1.83%--ip_forward
|
--1.82%--ip_forward_finish
|
--1.82%--ip_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.77%--neigh_event_send
|
--1.77%--__neigh_event_send
|
--1.74%--_raw_write_lock_bh
queued_write_lock
queued_write_lock_slowpath
|
--1.71%--queued_spin_lock_slowpath
1.85% 0.00% ksoftirqd/38 [kernel.vmlinux] [k]
ip_rcv_finish
|
--1.85%--ip_rcv_finish
|
--1.83%--ip_forward
|
--1.82%--ip_forward_finish
|
--1.82%--ip_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.77%--neigh_event_send
|
--1.77%--__neigh_event_send
|
--1.74%--_raw_write_lock_bh
queued_write_lock
queued_write_lock_slowpath
|
--1.71%--queued_spin_lock_slowpath
1.85% 0.00% ksoftirqd/22 [kernel.vmlinux] [k] ip_rcv
|
--1.85%--ip_rcv
|
--1.85%--ip_rcv_finish
|
--1.83%--ip_forward
|
--1.82%--ip_forward_finish
|
--1.82%--ip_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.77%--neigh_event_send
|
--1.77%--__neigh_event_send
|
--1.73%--_raw_write_lock_bh
queued_write_lock
queued_write_lock_slowpath
|
--1.70%--queued_spin_lock_slowpath
1.83% 0.00% ksoftirqd/9 [kernel.vmlinux] [k]
ip_forward
|
--1.83%--ip_forward
|
--1.82%--ip_forward_finish
|
--1.82%--ip_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.77%--neigh_event_send
|
--1.77%--__neigh_event_send
|
--1.74%--_raw_write_lock_bh
queued_write_lock
queued_write_lock_slowpath
|
--1.70%--queued_spin_lock_slowpath
1.82% 0.00% ksoftirqd/35 [kernel.vmlinux] [k]
ip_output
|
--1.82%--ip_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.77%--neigh_event_send
|
--1.77%--__neigh_event_send
|
--1.74%--_raw_write_lock_bh
queued_write_lock
queued_write_lock_slowpath
|
--1.71%--queued_spin_lock_slowpath
1.82% 0.00% ksoftirqd/38 [kernel.vmlinux] [k]
ip_finish_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.77%--neigh_event_send
|
--1.77%--__neigh_event_send
|
--1.74%--_raw_write_lock_bh
queued_write_lock
queued_write_lock_slowpath
|
--1.71%--queued_spin_lock_slowpath
1.82% 0.00% ksoftirqd/37 [kernel.vmlinux] [k]
ip_forward_finish
|
--1.82%--ip_forward_finish
ip_output
|
--1.82%--ip_finish_output
|
--1.82%--ip_finish_output2
|
--1.79%--neigh_resolve_output
|
--1.76%--neigh_event_send
__neigh_event_send
|
--1.73%--_raw_write_lock_bh
queued_write_lock
queued_write_lock_slowpath
|
--1.70%--queued_spin_lock_slowpath
W dniu 2017-08-16 o 09:42, Julian Anastasov pisze:
> Hello,
>
> On Tue, 15 Aug 2017, Eric Dumazet wrote:
>
>> It must be possible to add a fast path without locks.
>>
>> (say if jiffies has not changed before last state change)
> New day - new idea. Something like this? But it
> has bug: without checking neigh->dead under lock we don't
> have the right to access neigh->parms, it can be destroyed
> immediately by neigh_release->neigh_destroy->neigh_parms_put->
> neigh_parms_destroy->kfree. Not sure, may be kfree_rcu can help
> for this...
>
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index 9816df2..f52763c 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -428,10 +428,10 @@ static inline int neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
> {
> unsigned long now = jiffies;
>
> - if (neigh->used != now)
> - neigh->used = now;
> if (!(neigh->nud_state&(NUD_CONNECTED|NUD_DELAY|NUD_PROBE)))
> return __neigh_event_send(neigh, skb);
> + if (neigh->used != now)
> + neigh->used = now;
> return 0;
> }
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 16a1a4c..52a8718 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -991,8 +991,18 @@ static void neigh_timer_handler(unsigned long arg)
>
> int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
> {
> - int rc;
> bool immediate_probe = false;
> + unsigned long now = jiffies;
> + int rc;
> +
> + if (neigh->used != now) {
> + neigh->used = now;
> + } else if (neigh->nud_state == NUD_INCOMPLETE &&
> + (!skb || neigh->arp_queue_len_bytes + skb->truesize >
> + NEIGH_VAR(neigh->parms, QUEUE_LEN_BYTES))) {
> + kfree_skb(skb);
> + return 1;
> + }
>
> write_lock_bh(&neigh->lock);
>
> @@ -1005,7 +1015,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
> if (!(neigh->nud_state & (NUD_STALE | NUD_INCOMPLETE))) {
> if (NEIGH_VAR(neigh->parms, MCAST_PROBES) +
> NEIGH_VAR(neigh->parms, APP_PROBES)) {
> - unsigned long next, now = jiffies;
> + unsigned long next;
>
> atomic_set(&neigh->probes,
> NEIGH_VAR(neigh->parms, UCAST_PROBES));
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
>
^ permalink raw reply
* Re: [PATCH] net/mlx4: fix spelling mistake: "availible" -> "available"
From: Leon Romanovsky @ 2017-08-16 10:01 UTC (permalink / raw)
To: Colin King
Cc: Tariq Toukan, netdev, linux-rdma, kernel-janitors, linux-kernel
In-Reply-To: <20170816090511.10463-1-colin.king@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 540 bytes --]
On Wed, Aug 16, 2017 at 10:05:11AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistakes in the mlx4 driver
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/cmd.c | 16 ++++++++--------
> drivers/net/ethernet/mellanox/mlx4/fw_qos.c | 6 +++---
> drivers/net/ethernet/mellanox/mlx4/fw_qos.h | 10 +++++-----
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] net/i40e: use cpumask_copy() for assigning cpumask
From: Stefano Brivio @ 2017-08-16 9:52 UTC (permalink / raw)
To: Juergen Gross
Cc: linux-kernel, netdev, intel-wired-lan, jeffrey.t.kirsher, stable
In-Reply-To: <20170812160946.2278-1-jgross@suse.com>
Hi Juergen,
On Sat, 12 Aug 2017 18:09:46 +0200
Juergen Gross <jgross@suse.com> wrote:
> Using direct assignment for a cpumask is wrong, cpumask_copy() should
> be used instead.
Perhaps a Fixes: tag might be helpful here, such as:
Fixes: 96db776a3682 ("i40e/i40evf: fix interrupt affinity bug")
as I reported in my other (late) patch:
http://marc.info/?l=linux-netdev&m=150279303222066&w=2
> Cc: stable@vger.kernel.org
and maybe also an indication about which versions this applies to, such as
Cc: stable@vger.kernel.org # 4.10+
In general, feel free to copy from my commit message.
These comments also apply to:
[PATCH] net/i40evf: use cpumask_copy() for assigning cpumask
Thanks,
--
Stefano
^ permalink raw reply
* Re: [PATCH v3] openvswitch: enable NSH support
From: Yang, Yi @ 2017-08-16 9:31 UTC (permalink / raw)
To: Jiri Benc; +Cc: netdev, e, dev
In-Reply-To: <20170816111921.7a039be5@griffin>
On Wed, Aug 16, 2017 at 11:19:21AM +0200, Jiri Benc wrote:
> Please always CC reviewers of the previous versions, thanks.
Jiri, thank you for quick review. Sorry, I made a mistake on
sending and missed all the CCs, will indeed do this in next version.
> > + __be16 ver_flags_len;
> > + u8 md_type;
> > + u8 next_proto;
> > + __be32 path_hdr;
> > + u8 metadata[NSH_M_TYPE2_MAX_LEN-8];
> > +};
>
> Please get rid of this struct. It's a copy of struct nsh_hdr with some
> space added to the bottom.
>
> One of the options (though maybe not the best one, feel free to come up
> with something better) is to change struct nsh_md1_ctx to:
>
> struct nsh_md1_ctx {
> __be32 context[];
> };
>
> and change struct push_nsh_para:
>
> struct push_nsh_para {
> struct nsh_hdr hdr;
> u8 metadata[NSH_M_TYPE2_MAX_LEN-8];
> };
>
> Another option (a better one, though a bit more work) is to get rid of
> push_nsh_para completely and just pass a properly allocated nsh_hdr
> around. Introduce macros and/or functions to help with the allocation.
>
Yeah, good point, we can use dynamic allocation and struct nsh_hdr * to
handle this.
> > +static inline struct nsh_md1_ctx *nsh_md1_ctx(struct nsh_hdr *nsh)
> > +{
> > + return &nsh->md1;
> > +}
> > +
> > +static inline struct nsh_md2_tlv *nsh_md2_ctx(struct nsh_hdr *nsh)
> > +{
> > + return nsh->md2;
> > +}
>
> These are unused, please remove them.
Will remove them, userspace does use them.
>
> > --- a/include/uapi/linux/openvswitch.h
> > +++ b/include/uapi/linux/openvswitch.h
> [...]
> > +#define NSH_MD1_CONTEXT_SIZE 4
>
> Please move this to nsh.h and use it there, too, instead of the open
> coded 4.
ovs code is very ugly, it will convert array[4] in
datapath/linux/compat/include/linux/openvswitch.h to other struct, I
have to change context[4] to such format :-), we can use 4 here for
Linux kernel.
>
> > +static int push_nsh(struct sk_buff *skb, struct sw_flow_key *key,
> > + const struct push_nsh_para *pnp)
> > +{
> > + struct nsh_hdr *nsh;
> > + size_t length = ((ntohs(pnp->ver_flags_len) & NSH_LEN_MASK)
> > + >> NSH_LEN_SHIFT) << 2;
>
> Once push_nsh_para is removed/changed, this can be changed to a call to
> nsh_hdr_len.
Yes, will do that way.
>
> > + flags = (ntohs(nsh->ver_flags_len) & NSH_FLAGS_MASK) >>
> > + NSH_FLAGS_SHIFT;
>
> nsh_get_flags
Missed this one :-)
>
> > + case OVS_KEY_ATTR_NSH: {
> > + struct ovs_key_nsh nsh;
> > + struct ovs_key_nsh nsh_mask;
> > + size_t size = nla_len(a) / 2;
> > + struct nlattr attr[1 + DIV_ROUND_UP(sizeof(struct ovs_key_ipv6)
> > + , sizeof(struct nlattr))];
> > + struct nlattr mask[1 + DIV_ROUND_UP(sizeof(struct ovs_key_ipv6)
> > + , sizeof(struct nlattr))];
> > +
> > + attr->nla_type = nla_type(a);
> > + mask->nla_type = attr->nla_type;
> > + attr->nla_len = NLA_HDRLEN + size;
> > + mask->nla_len = attr->nla_len;
> > + memcpy(attr + 1, (char *)(a + 1), size);
> > + memcpy(mask + 1, (char *)(a + 1) + size, size);
>
> This is too hacky. Please find a better way to handle this.
>
> One option is to create a struct with struct nlattr as the first member
> followed by a char buffer. Still not nice but at least it's clear
> what's the intent.
The issue is nested attributes only can use this way, nested attribute
for SET_MASKED is very special, we have to handle it specially.
>
> > +static int parse_nsh(struct sk_buff *skb, struct sw_flow_key *key)
> > +{
> > + struct nsh_hdr *nsh = (struct nsh_hdr *)skb_network_header(skb);
> > + u8 version, length;
> > + u32 path_hdr;
> > + int i;
> > +
> > + memset(&key->nsh, 0, sizeof(struct ovs_key_nsh));
> > + version = nsh_get_ver(nsh);
> > + length = nsh_get_len(nsh);
> > +
> > + key->nsh.flags = nsh_get_flags(nsh);
> > + key->nsh.mdtype = nsh->md_type;
> > + key->nsh.np = nsh->next_proto;
> > + path_hdr = ntohl(nsh->path_hdr);
>
> The path_hdr variable is unused.
Will remove it.
>
> > + key->nsh.path_hdr = nsh->path_hdr;
> > + switch (key->nsh.mdtype) {
> > + case NSH_M_TYPE1:
> > + if ((length << 2) != NSH_M_TYPE1_LEN)
>
> Why length << 2?
len in NSH header is number of 4 octets, so need to multiply 4.
>
> > + return -EINVAL;
> > +
> > + for (i = 0; i < 4; i++)
>
> NSH_MD1_CONTEXT_SIZE
Ok
>
> > + key->nsh.context[i] = nsh->md1.context[i];
> > +
> > + break;
>
> Will go through the rest later. Feel free to send a new version
> meanwhile.
>
> Thanks,
>
> Jiri
Thank you so much for your comments, will work out new version ASAP.
^ permalink raw reply
* Re: [PATCH] net/mlx4: fix spelling mistake: "availible" -> "available"
From: Yuval Shaia @ 2017-08-16 9:42 UTC (permalink / raw)
To: Colin King
Cc: Tariq Toukan, netdev, linux-rdma, kernel-janitors, linux-kernel
In-Reply-To: <20170816090511.10463-1-colin.king@canonical.com>
On Wed, Aug 16, 2017 at 10:05:11AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistakes in the mlx4 driver
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/cmd.c | 16 ++++++++--------
> drivers/net/ethernet/mellanox/mlx4/fw_qos.c | 6 +++---
> drivers/net/ethernet/mellanox/mlx4/fw_qos.h | 10 +++++-----
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
> index 674773b28b2e..6309389b09a7 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
> @@ -1958,19 +1958,19 @@ static void mlx4_allocate_port_vpps(struct mlx4_dev *dev, int port)
> int i;
> int err;
> int num_vfs;
> - u16 availible_vpp;
> + u16 available_vpp;
> u8 vpp_param[MLX4_NUM_UP];
> struct mlx4_qos_manager *port_qos;
> struct mlx4_priv *priv = mlx4_priv(dev);
>
> - err = mlx4_ALLOCATE_VPP_get(dev, port, &availible_vpp, vpp_param);
> + err = mlx4_ALLOCATE_VPP_get(dev, port, &available_vpp, vpp_param);
> if (err) {
> - mlx4_info(dev, "Failed query availible VPPs\n");
> + mlx4_info(dev, "Failed query available VPPs\n");
> return;
> }
>
> port_qos = &priv->mfunc.master.qos_ctl[port];
> - num_vfs = (availible_vpp /
> + num_vfs = (available_vpp /
> bitmap_weight(port_qos->priority_bm, MLX4_NUM_UP));
>
> for (i = 0; i < MLX4_NUM_UP; i++) {
> @@ -1985,14 +1985,14 @@ static void mlx4_allocate_port_vpps(struct mlx4_dev *dev, int port)
> }
>
> /* Query actual allocated VPP, just to make sure */
> - err = mlx4_ALLOCATE_VPP_get(dev, port, &availible_vpp, vpp_param);
> + err = mlx4_ALLOCATE_VPP_get(dev, port, &available_vpp, vpp_param);
> if (err) {
> - mlx4_info(dev, "Failed query availible VPPs\n");
> + mlx4_info(dev, "Failed query available VPPs\n");
> return;
> }
>
> port_qos->num_of_qos_vfs = num_vfs;
> - mlx4_dbg(dev, "Port %d Availible VPPs %d\n", port, availible_vpp);
> + mlx4_dbg(dev, "Port %d Availible VPPs %d\n", port, available_vpp);
>
> for (i = 0; i < MLX4_NUM_UP; i++)
> mlx4_dbg(dev, "Port %d UP %d Allocated %d VPPs\n", port, i,
> @@ -2891,7 +2891,7 @@ static int mlx4_set_vport_qos(struct mlx4_priv *priv, int slave, int port,
> memset(vpp_qos, 0, sizeof(struct mlx4_vport_qos_param) * MLX4_NUM_UP);
>
> if (slave > port_qos->num_of_qos_vfs) {
> - mlx4_info(dev, "No availible VPP resources for this VF\n");
> + mlx4_info(dev, "No available VPP resources for this VF\n");
> return -EINVAL;
> }
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/fw_qos.c b/drivers/net/ethernet/mellanox/mlx4/fw_qos.c
> index 8f2fde0487c4..3a09d7122d3b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/fw_qos.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/fw_qos.c
> @@ -65,7 +65,7 @@ struct mlx4_set_port_scheduler_context {
>
> /* Granular Qos (per VF) section */
> struct mlx4_alloc_vpp_param {
> - __be32 availible_vpp;
> + __be32 available_vpp;
> __be32 vpp_p_up[MLX4_NUM_UP];
> };
>
> @@ -157,7 +157,7 @@ int mlx4_SET_PORT_SCHEDULER(struct mlx4_dev *dev, u8 port, u8 *tc_tx_bw,
> EXPORT_SYMBOL(mlx4_SET_PORT_SCHEDULER);
>
> int mlx4_ALLOCATE_VPP_get(struct mlx4_dev *dev, u8 port,
> - u16 *availible_vpp, u8 *vpp_p_up)
> + u16 *available_vpp, u8 *vpp_p_up)
> {
> int i;
> int err;
> @@ -179,7 +179,7 @@ int mlx4_ALLOCATE_VPP_get(struct mlx4_dev *dev, u8 port,
> goto out;
>
> /* Total number of supported VPPs */
> - *availible_vpp = (u16)be32_to_cpu(out_param->availible_vpp);
> + *available_vpp = (u16)be32_to_cpu(out_param->available_vpp);
>
> for (i = 0; i < MLX4_NUM_UP; i++)
> vpp_p_up[i] = (u8)be32_to_cpu(out_param->vpp_p_up[i]);
> diff --git a/drivers/net/ethernet/mellanox/mlx4/fw_qos.h b/drivers/net/ethernet/mellanox/mlx4/fw_qos.h
> index ac1f331878e6..582997577a04 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/fw_qos.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/fw_qos.h
> @@ -84,23 +84,23 @@ int mlx4_SET_PORT_PRIO2TC(struct mlx4_dev *dev, u8 port, u8 *prio2tc);
> int mlx4_SET_PORT_SCHEDULER(struct mlx4_dev *dev, u8 port, u8 *tc_tx_bw,
> u8 *pg, u16 *ratelimit);
> /**
> - * mlx4_ALLOCATE_VPP_get - Query port VPP availible resources and allocation.
> - * Before distribution of VPPs to priorities, only availible_vpp is returned.
> + * mlx4_ALLOCATE_VPP_get - Query port VPP available resources and allocation.
> + * Before distribution of VPPs to priorities, only available_vpp is returned.
> * After initialization it returns the distribution of VPPs among priorities.
> *
> * @dev: mlx4_dev.
> * @port: Physical port number.
> - * @availible_vpp: Pointer to variable where number of availible VPPs is stored
> + * @available_vpp: Pointer to variable where number of available VPPs is stored
> * @vpp_p_up: Distribution of VPPs to priorities is stored in this array
> *
> * Returns 0 on success or a negative mlx4_core errno code.
> **/
> int mlx4_ALLOCATE_VPP_get(struct mlx4_dev *dev, u8 port,
> - u16 *availible_vpp, u8 *vpp_p_up);
> + u16 *available_vpp, u8 *vpp_p_up);
> /**
> * mlx4_ALLOCATE_VPP_set - Distribution of VPPs among differnt priorities.
> * The total number of VPPs assigned to all for a port must not exceed
> - * the value reported by availible_vpp in mlx4_ALLOCATE_VPP_get.
> + * the value reported by available_vpp in mlx4_ALLOCATE_VPP_get.
> * VPP allocation is allowed only after the port type has been set,
> * and while no QPs are open for this port.
> *
Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
(I'm using setlocal spell spelllang=en to avoid such spelling mistakes)
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net 2/2] net: ixgbe: Use new IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING flag
From: Ding Tianhong @ 2017-08-16 9:41 UTC (permalink / raw)
To: davem, jeffrey.t.kirsher, keescook, linux-kernel, sparclinux,
intel-wired-lan, alexander.duyck, netdev
Cc: Ding Tianhong
In-Reply-To: <1502876507-9360-1-git-send-email-dingtianhong@huawei.com>
The ixgbe driver use the compile check to determine if it can
send TLPs to Root Port with the Relaxed Ordering Attribute set,
this is too inconvenient, now the new flag PCI_DEV_FLAGS_NO_RELAXED_ORDERING
has been added to the kernel and we could check the bit4 in the PCIe
Davice Control register to determine whether we should use the Relaxed
Ordering Attributes or not, so we add a new flag which called
IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING to the ixgbe driver, it will
be set if the Root Port couldn't deal the upstream TLPs with Relaxed
Ordering Attribute, then the driver could know what to do next.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c | 37 ++++++++++++-------------
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 32 +++++++++++----------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 ++++++++++++
4 files changed, 53 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index dd55787..50e0553 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -621,6 +621,7 @@ struct ixgbe_adapter {
#define IXGBE_FLAG2_EEE_CAPABLE BIT(14)
#define IXGBE_FLAG2_EEE_ENABLED BIT(15)
#define IXGBE_FLAG2_RX_LEGACY BIT(16)
+#define IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING BIT(17)
/* Tx fast path data */
int num_tx_queues;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
index 523f9d0..0727a30 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
@@ -175,31 +175,30 @@ static s32 ixgbe_init_phy_ops_82598(struct ixgbe_hw *hw)
**/
static s32 ixgbe_start_hw_82598(struct ixgbe_hw *hw)
{
-#ifndef CONFIG_SPARC
- u32 regval;
- u32 i;
-#endif
+ u32 regval, i;
s32 ret_val;
+ struct ixgbe_adapter *adapter = hw->back;
ret_val = ixgbe_start_hw_generic(hw);
-#ifndef CONFIG_SPARC
- /* Disable relaxed ordering */
- for (i = 0; ((i < hw->mac.max_tx_queues) &&
- (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
- regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
- regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
- IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
- }
+ if (adapter->flags2 & IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING) {
+ /* Disable relaxed ordering */
+ for (i = 0; ((i < hw->mac.max_tx_queues) &&
+ (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
+ regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
+ regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
+ IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
+ }
- for (i = 0; ((i < hw->mac.max_rx_queues) &&
- (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
- regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
- regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
- IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
- IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+ for (i = 0; ((i < hw->mac.max_rx_queues) &&
+ (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
+ regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
+ regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
+ IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
+ IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+ }
}
-#endif
+
if (ret_val)
return ret_val;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index d4933d2..2473c0b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -342,6 +342,7 @@ s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw)
s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
{
u32 i;
+ struct ixgbe_adapter *adapter = hw->back;
/* Clear the rate limiters */
for (i = 0; i < hw->mac.max_tx_queues; i++) {
@@ -350,25 +351,26 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
}
IXGBE_WRITE_FLUSH(hw);
-#ifndef CONFIG_SPARC
- /* Disable relaxed ordering */
- for (i = 0; i < hw->mac.max_tx_queues; i++) {
- u32 regval;
+ if (adapter->flags2 & IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING) {
+ /* Disable relaxed ordering */
+ for (i = 0; i < hw->mac.max_tx_queues; i++) {
+ u32 regval;
- regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
- regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
- IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
- }
+ regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
+ regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
+ IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
+ }
- for (i = 0; i < hw->mac.max_rx_queues; i++) {
- u32 regval;
+ for (i = 0; i < hw->mac.max_rx_queues; i++) {
+ u32 regval;
- regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
- regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
- IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
- IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+ regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
+ regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
+ IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
+ IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+ }
}
-#endif
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f1dbdf2..f576be7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10081,6 +10081,23 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_ioremap;
}
+ /* If possible, we use PCIe Relaxed Ordering Attribute to deliver
+ * Ingress Packet Data to Free List Buffers in order to allow for
+ * chipset performance optimizations between the Root Complex and
+ * Memory Controllers. (Messages to the associated Ingress Queue
+ * notifying new Packet Placement in the Free Lists Buffers will be
+ * send without the Relaxed Ordering Attribute thus guaranteeing that
+ * all preceding PCIe Transaction Layer Packets will be processed
+ * first.) But some Root Complexes have various issues with Upstream
+ * Transaction Layer Packets with the Relaxed Ordering Attribute set.
+ * The PCIe devices which under the Root Complexes will be cleared the
+ * Relaxed Ordering bit in the configuration space, So we check our
+ * PCIe configuration space to see if it's flagged with advice against
+ * using Relaxed Ordering.
+ */
+ if (!pcie_relaxed_ordering_enabled(pdev))
+ adapter->flags2 |= IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING;
+
netdev->netdev_ops = &ixgbe_netdev_ops;
ixgbe_set_ethtool_ops(netdev);
netdev->watchdog_timeo = 5 * HZ;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 1/2] Revert commit 1a8b6d76dc5b ("net:add one common config...")
From: Ding Tianhong @ 2017-08-16 9:41 UTC (permalink / raw)
To: davem, jeffrey.t.kirsher, keescook, linux-kernel, sparclinux,
intel-wired-lan, alexander.duyck, netdev
Cc: Ding Tianhong
In-Reply-To: <1502876507-9360-1-git-send-email-dingtianhong@huawei.com>
The new flag PCI_DEV_FLAGS_NO_RELAXED_ORDERING has been added
to indicate that Relaxed Ordering Attributes (RO) should not
be used for Transaction Layer Packets (TLP) targeted toward
these affected Root Port, it will clear the bit4 in the PCIe
Device Control register, so the PCIe device drivers could
query PCIe configuration space to determine if it can send
TLPs to Root Port with the Relaxed Ordering Attributes set.
With this new flag we don't need the config ARCH_WANT_RELAX_ORDER
to control the Relaxed Ordering Attributes for the ixgbe drivers
just like the commit 1a8b6d76dc5b ("net:add one common config...") did,
so revert this commit.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
arch/Kconfig | 3 ---
arch/sparc/Kconfig | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 2 +-
3 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 21d0089..00cfc63 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -928,9 +928,6 @@ config STRICT_MODULE_RWX
and non-text memory will be made non-executable. This provides
protection against certain security exploits (e.g. writing to text)
-config ARCH_WANT_RELAX_ORDER
- bool
-
config REFCOUNT_FULL
bool "Perform full reference count validation at the expense of speed"
help
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index a4a6261..987a575 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -44,7 +44,6 @@ config SPARC
select ARCH_HAS_SG_CHAIN
select CPU_NO_EFFICIENT_FFS
select LOCKDEP_SMALL if LOCKDEP
- select ARCH_WANT_RELAX_ORDER
config SPARC32
def_bool !64BIT
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 4e35e70..d4933d2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -350,7 +350,7 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
}
IXGBE_WRITE_FLUSH(hw);
-#ifndef CONFIG_ARCH_WANT_RELAX_ORDER
+#ifndef CONFIG_SPARC
/* Disable relaxed ordering */
for (i = 0; i < hw->mac.max_tx_queues; i++) {
u32 regval;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 0/2] net: ixgbe: Use new flag to disable Relaxed Ordering
From: Ding Tianhong @ 2017-08-16 9:41 UTC (permalink / raw)
To: davem, jeffrey.t.kirsher, keescook, linux-kernel, sparclinux,
intel-wired-lan, alexander.duyck, netdev
Cc: Ding Tianhong
The new flag PCI_DEV_FLAGS_NO_RELAXED_ORDERING has been added
to indicate that Relaxed Ordering Attributes (RO) should not
be used for Transaction Layer Packets (TLP) targeted toward
these affected Root Port, it will clear the bit4 in the PCIe
Device Control register, so the PCIe device drivers could
query PCIe configuration space to determine if it can send
TLPs to Root Port with the Relaxed Ordering Attributes set.
The ixgbe driver could use this flag to determine if it can
send TLPs to Root Port with the Relaxed Ordering Attributes set.
Ding Tianhong (2):
Revert commit 1a8b6d76dc5b ("net:add one common config...")
net: ixgbe: Use new IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING flag
arch/Kconfig | 3 --
arch/sparc/Kconfig | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c | 37 ++++++++++++-------------
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 32 +++++++++++----------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 ++++++++++++
6 files changed, 53 insertions(+), 38 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
From: Christian König @ 2017-08-16 9:41 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Chris Mi, netdev
In-Reply-To: <20170816093144.GE1868@nanopsycho>
Am 16.08.2017 um 11:31 schrieb Jiri Pirko:
> [SNIP]
> I don't. It is an API change, maintainers of the individual drivers are
> not expected to review the patches like this.
Yeah, completely agree.
>> If yes then it somehow makes sense to send the patch bit by bit, if no then
>> it doesn't seem to make to much sense to CC them all individually.
>>
>>>>>> I've never read the bsg code before, but that's certainly not correct. And
>>>>>> that incorrect pattern repeats over and over again in this code.
>>>>>>
>>>>>> Apart from that why the heck do you want to allocate more than 1<<31 handles?
>>>>> tc action indexes for example. That is part of this patchset.
>>>> Well, let me refine the question: Why does tc action indexes need more than
>>>> 31 bits? From an outside view that looks like pure overkill.
>>> That is current state, uapi. We have to live with it.
>> Is the range to allocate from part of the uapi or what is the issue here?
> Yes.
A bit strange uapi design, but ok in this case that change actually
makes sense.
>> If the issue is that userspace can specify the handle then I suggest that you
>> use the radix tree directly instead of the idr wrapper around it.
> But why? idr is exactly the tool we need. Only signed int does not suit
> us. In fact, it does not make sense idr is using signed int when it
> uses radix tree with unsigned long under the hood.
Well it always depends on what you do and how to use it.
In amdgpu for example for have very very short lived objects and only
few of them are active at the same time.
The solution was not to use and idr, but rather 64bit identifiers and a
ring buffer with the last 128 entries.
But in your case changing the idr calling convention actually makes
sense (at least from the tn mile high view), feel free to add an
Acked-by: Christian König <christian.koenig@amd.com> on it.
Regards,
Christian.
^ permalink raw reply
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
From: Jiri Pirko @ 2017-08-16 9:31 UTC (permalink / raw)
To: Christian König; +Cc: Chris Mi, netdev
In-Reply-To: <5867bd36-b758-f364-062a-2b612e8c7318@amd.com>
Wed, Aug 16, 2017 at 10:55:56AM CEST, christian.koenig@amd.com wrote:
>Am 16.08.2017 um 10:39 schrieb Jiri Pirko:
>> Wed, Aug 16, 2017 at 10:31:35AM CEST, christian.koenig@amd.com wrote:
>> > Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
>> > > Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
>> > > > Am 16.08.2017 um 04:12 schrieb Chris Mi:
>> > > > > Using current TC code, it is very slow to insert a lot of rules.
>> > > > >
>> > > > > In order to improve the rules update rate in TC,
>> > > > > we introduced the following two changes:
>> > > > > 1) changed cls_flower to use IDR to manage the filters.
>> > > > > 2) changed all act_xxx modules to use IDR instead of
>> > > > > a small hash table
>> > > > >
>> > > > > But IDR has a limitation that it uses int. TC handle uses u32.
>> > > > > To make sure there is no regression, we also changed IDR to use
>> > > > > unsigned long. All clients of IDR are changed to use new IDR API.
>> > > > WOW, wait a second. The idr change is touching a lot of drivers and to be
>> > > > honest doesn't looks correct at all.
>> > > >
>> > > > Just look at the first chunk of your modification:
>> > > > > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>> > > > > mutex_lock(&bsg_mutex);
>> > > > > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
>> > > > > - if (ret < 0) {
>> > > > > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
>> > > > > + GFP_KERNEL);
>> > > > > + if (ret) {
>> > > > > if (ret == -ENOSPC) {
>> > > > > printk(KERN_ERR "bsg: too many bsg devices\n");
>> > > > > ret = -EINVAL;
>> > > > The condition "if (ret)" will now always be true after the first allocation
>> > > > and so we always run into the error handling after that.
>> > > On success, idr_alloc returns 0.
>> > Ah, I see. You change the idr_alloc to return the resulting index as separate
>> > parameter.
>> >
>> > You should explicit note that in the commit message, cause that is something
>> > easily overlooked.
>> >
>> > In general I strongly suggest to add a separate interface for allocating
>> > unsigned long handles, use that for the while being and then move the
>> > existing drivers over bit by bit.
>> >
>> > A single patch which touches so many different driver is practically
>> > impossible to review consequently.
>> Understood. I think is is good to avoid having some "idr_alloc2". That
>> is why I suggested to do this in one go, to avoid "idr_alloc2" and then
>> patch to rename "idr_alloc2" to "idr_alloc" once nobody uses the original
>> "idr_alloc". In fact, if you do it driver, by driver, the review burden
>> would be the same, probably even bigger, you'll just have 100+ patches.
>> Why would it help?
>
>Because it would give each maintainer only the part of the change he is
>interested in.
>
>Current status of this change is that you send a mail with nearly 300 people
>on CC.
That was a mistake to cc all.
>
>Do you really expect to get an reviewed-by or acked-by on this single patch
>from all of them?
I don't. It is an API change, maintainers of the individual drivers are
not expected to review the patches like this.
>
>If yes then it somehow makes sense to send the patch bit by bit, if no then
>it doesn't seem to make to much sense to CC them all individually.
>
>> > > > I've never read the bsg code before, but that's certainly not correct. And
>> > > > that incorrect pattern repeats over and over again in this code.
>> > > >
>> > > > Apart from that why the heck do you want to allocate more than 1<<31 handles?
>> > > tc action indexes for example. That is part of this patchset.
>> > Well, let me refine the question: Why does tc action indexes need more than
>> > 31 bits? From an outside view that looks like pure overkill.
>> That is current state, uapi. We have to live with it.
>
>Is the range to allocate from part of the uapi or what is the issue here?
Yes.
>
>If the issue is that userspace can specify the handle then I suggest that you
>use the radix tree directly instead of the idr wrapper around it.
But why? idr is exactly the tool we need. Only signed int does not suit
us. In fact, it does not make sense idr is using signed int when it
uses radix tree with unsigned long under the hood.
>
>Regards,
>Christian.
^ permalink raw reply
* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Paolo Abeni @ 2017-08-16 9:28 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Matthew Dawson, Network Development, Macieira, Thiago
In-Reply-To: <CAF=yD-+jBim==ktwYUNAFkn-N4eabqgNME8NXM77v9-U_bfWPg@mail.gmail.com>
On Tue, 2017-08-15 at 13:00 -0400, Willem de Bruijn wrote:
> > There is another difference between reading sk_peek_offset in the
> > caller or in __skb_try_recv_from_queue. The latter is called repeatedly
> > when it returns NULL. Each call can modify *off. I believe that it needs
> > to restart with _off at sk->sk_peek_off each time, as it restarts from the
> > head of the queue each time.
>
> I made a mistake here. *off is not updated when returning NULL.
>
> In that case, it is better to read sk_peek_offset once, than to read
> it each time __skb_try_recv_from_queue is entered.
If I read the above correctly, you are arguining in favor of the
addittional flag version, right?
Regarding the MSG flag exaustion, there are a bunch of flags defined
but apparently unused (MSG_FIN, MSG_SYN, MSG_RST) since long time (if
I'm not too low on coffee).
We can shadow one of them (or ev. drop the above defines, if really
unused).
I think that the MSG_PEEK_OFF should be explicitly cleared in
sk_peek_offset() when the sk_peek_off is negative, to avoid beeing
fooled by stray bits passed by the us, like in the following:
---
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8b13db5163cc..3b7f53b9cc08 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -286,6 +286,7 @@ struct ucred {
#define MSG_SENDPAGE_NOTLAST 0x20000 /* sendpage() internal : not the last page */
#define MSG_BATCH 0x40000 /* sendmmsg(): more messages coming */
#define MSG_EOF MSG_FIN
+#define MSG_PEEK_OFF MSG_FIN /* Peeking with offset for datagram sockets */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
#define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file
diff --git a/include/net/sock.h b/include/net/sock.h
index 7c0632c7e870..452f9aac2e6a 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -504,12 +504,17 @@ enum sk_pacing {
int sk_set_peek_off(struct sock *sk, int val);
-static inline int sk_peek_offset(struct sock *sk, int flags)
+static inline int sk_peek_offset(struct sock *sk, int *flags)
{
- if (unlikely(flags & MSG_PEEK)) {
+ if (unlikely(*flags & MSG_PEEK)) {
s32 off = READ_ONCE(sk->sk_peek_off);
- if (off >= 0)
+ if (off >= 0) {
+ *flags |= MSG_PEEK_OFF;
return off;
+ }
+
+ /* clear evental stray bits in the user-provided bitmask */
+ *flags &= ~MSG_PEEK_OFF;
}
return 0;
diff --git a/net/core/datagram.c b/net/core/datagram.c
index ee5647bd91b3..91e1d014d64c 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -175,8 +175,8 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
*last = queue->prev;
skb_queue_walk(queue, skb) {
if (flags & MSG_PEEK) {
- if (_off >= skb->len && (skb->len || _off ||
- skb->peeked)) {
+ if (flags & MSG_PEEK_OFF && _off >= skb->len &&
+ (skb->len || _off || skb->peeked)) {
_off -= skb->len;
continue;
}
diff --git a/net/core/sock.c b/net/core/sock.c
index ac2a404c73eb..0c123540b02f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2408,9 +2408,7 @@ EXPORT_SYMBOL(__sk_mem_reclaim);
int sk_set_peek_off(struct sock *sk, int val)
{
- if (val < 0)
- return -EINVAL;
-
+ /* a negative value will disable peeking with offset */
sk->sk_peek_off = val;
return 0;
}
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index a7c804f73990..7e1bcd3500f4 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1574,7 +1574,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
return ip_recv_error(sk, msg, len, addr_len);
try_again:
- peeking = off = sk_peek_offset(sk, flags);
+ peeking = off = sk_peek_offset(sk, &flags);
skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
if (!skb)
return err;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 578142b7ca3e..86fb4ff8934c 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -362,7 +362,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
return ipv6_recv_rxpmtu(sk, msg, len, addr_len);
try_again:
- peeking = off = sk_peek_offset(sk, flags);
+ peeking = off = sk_peek_offset(sk, &flags);
skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
if (!skb)
return err;
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 7b52a380d710..06c740939d9d 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2124,7 +2124,7 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
do {
mutex_lock(&u->iolock);
- skip = sk_peek_offset(sk, flags);
+ skip = sk_peek_offset(sk, &flags);
skb = __skb_try_recv_datagram(sk, flags, NULL, &peeked, &skip,
&err, &last);
if (skb)
@@ -2304,10 +2304,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
*/
mutex_lock(&u->iolock);
- if (flags & MSG_PEEK)
- skip = sk_peek_offset(sk, flags);
- else
- skip = 0;
+ skip = sk_peek_offset(sk, &flags);
do {
int chunk;
^ permalink raw reply related
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy, linux-
In-Reply-To: <144b87a3-bbe4-a194-ed83-e54840d7c7c2@amd.com>
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply
* Re: [PATCH v3] openvswitch: enable NSH support
From: Jiri Benc @ 2017-08-16 9:19 UTC (permalink / raw)
To: Yi Yang; +Cc: netdev, Eric Garver
In-Reply-To: <1502861730-76203-1-git-send-email-yi.y.yang@intel.com>
Please always CC reviewers of the previous versions, thanks.
On Wed, 16 Aug 2017 13:35:30 +0800, Yi Yang wrote:
> v2->v3
> - Change OVS_KEY_ATTR_NSH to nested key to handle
> length-fixed attributes and length-variable
> attriubte more flexibly.
> - Remove struct ovs_action_push_nsh completely
> - Add code to handle nested attribute for SET_MASKED
> - Change PUSH_NSH to use the nested OVS_KEY_ATTR_NSH
> to transfer NSH header data.
> - Fix comments and coding style issues by Jiri and Eric
Thanks! I like this version. I think we're almost there. A few more
comments below.
> +struct push_nsh_para {
> + __be16 ver_flags_len;
> + u8 md_type;
> + u8 next_proto;
> + __be32 path_hdr;
> + u8 metadata[NSH_M_TYPE2_MAX_LEN-8];
> +};
Please get rid of this struct. It's a copy of struct nsh_hdr with some
space added to the bottom.
One of the options (though maybe not the best one, feel free to come up
with something better) is to change struct nsh_md1_ctx to:
struct nsh_md1_ctx {
__be32 context[];
};
and change struct push_nsh_para:
struct push_nsh_para {
struct nsh_hdr hdr;
u8 metadata[NSH_M_TYPE2_MAX_LEN-8];
};
Another option (a better one, though a bit more work) is to get rid of
push_nsh_para completely and just pass a properly allocated nsh_hdr
around. Introduce macros and/or functions to help with the allocation.
> +static inline struct nsh_md1_ctx *nsh_md1_ctx(struct nsh_hdr *nsh)
> +{
> + return &nsh->md1;
> +}
> +
> +static inline struct nsh_md2_tlv *nsh_md2_ctx(struct nsh_hdr *nsh)
> +{
> + return nsh->md2;
> +}
These are unused, please remove them.
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
[...]
> +#define NSH_MD1_CONTEXT_SIZE 4
Please move this to nsh.h and use it there, too, instead of the open
coded 4.
> +static int push_nsh(struct sk_buff *skb, struct sw_flow_key *key,
> + const struct push_nsh_para *pnp)
> +{
> + struct nsh_hdr *nsh;
> + size_t length = ((ntohs(pnp->ver_flags_len) & NSH_LEN_MASK)
> + >> NSH_LEN_SHIFT) << 2;
Once push_nsh_para is removed/changed, this can be changed to a call to
nsh_hdr_len.
> + flags = (ntohs(nsh->ver_flags_len) & NSH_FLAGS_MASK) >>
> + NSH_FLAGS_SHIFT;
nsh_get_flags
> + case OVS_KEY_ATTR_NSH: {
> + struct ovs_key_nsh nsh;
> + struct ovs_key_nsh nsh_mask;
> + size_t size = nla_len(a) / 2;
> + struct nlattr attr[1 + DIV_ROUND_UP(sizeof(struct ovs_key_ipv6)
> + , sizeof(struct nlattr))];
> + struct nlattr mask[1 + DIV_ROUND_UP(sizeof(struct ovs_key_ipv6)
> + , sizeof(struct nlattr))];
> +
> + attr->nla_type = nla_type(a);
> + mask->nla_type = attr->nla_type;
> + attr->nla_len = NLA_HDRLEN + size;
> + mask->nla_len = attr->nla_len;
> + memcpy(attr + 1, (char *)(a + 1), size);
> + memcpy(mask + 1, (char *)(a + 1) + size, size);
This is too hacky. Please find a better way to handle this.
One option is to create a struct with struct nlattr as the first member
followed by a char buffer. Still not nice but at least it's clear
what's the intent.
> +static int parse_nsh(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> + struct nsh_hdr *nsh = (struct nsh_hdr *)skb_network_header(skb);
> + u8 version, length;
> + u32 path_hdr;
> + int i;
> +
> + memset(&key->nsh, 0, sizeof(struct ovs_key_nsh));
> + version = nsh_get_ver(nsh);
> + length = nsh_get_len(nsh);
> +
> + key->nsh.flags = nsh_get_flags(nsh);
> + key->nsh.mdtype = nsh->md_type;
> + key->nsh.np = nsh->next_proto;
> + path_hdr = ntohl(nsh->path_hdr);
The path_hdr variable is unused.
> + key->nsh.path_hdr = nsh->path_hdr;
> + switch (key->nsh.mdtype) {
> + case NSH_M_TYPE1:
> + if ((length << 2) != NSH_M_TYPE1_LEN)
Why length << 2?
> + return -EINVAL;
> +
> + for (i = 0; i < 4; i++)
NSH_MD1_CONTEXT_SIZE
> + key->nsh.context[i] = nsh->md1.context[i];
> +
> + break;
Will go through the rest later. Feel free to send a new version
meanwhile.
Thanks,
Jiri
^ permalink raw reply
* Re: [PATCH net-next V2 1/3] tap: use build_skb() for small packet
From: Jason Wang @ 2017-08-16 9:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Eric Dumazet, davem, netdev, linux-kernel, kubakici
In-Reply-To: <3b24805d-b489-2dfc-f930-0518ba1a6ea0@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]
On 2017年08月16日 12:07, Jason Wang wrote:
>
>
> On 2017年08月16日 11:59, Michael S. Tsirkin wrote:
>> On Wed, Aug 16, 2017 at 11:57:51AM +0800, Jason Wang wrote:
>>>
>>> On 2017年08月16日 11:55, Michael S. Tsirkin wrote:
>>>> On Tue, Aug 15, 2017 at 08:45:20PM -0700, Eric Dumazet wrote:
>>>>> On Fri, 2017-08-11 at 19:41 +0800, Jason Wang wrote:
>>>>>> We use tun_alloc_skb() which calls sock_alloc_send_pskb() to
>>>>>> allocate
>>>>>> skb in the past. This socket based method is not suitable for high
>>>>>> speed userspace like virtualization which usually:
>>>>>>
>>>>>> - ignore sk_sndbuf (INT_MAX) and expect to receive the packet as
>>>>>> fast as
>>>>>> possible
>>>>>> - don't want to be block at sendmsg()
>>>>>>
>>>>>> To eliminate the above overheads, this patch tries to use
>>>>>> build_skb()
>>>>>> for small packet. We will do this only when the following conditions
>>>>>> are all met:
>>>>>>
>>>>>> - TAP instead of TUN
>>>>>> - sk_sndbuf is INT_MAX
>>>>>> - caller don't want to be blocked
>>>>>> - zerocopy is not used
>>>>>> - packet size is smaller enough to use build_skb()
>>>>>>
>>>>>> Pktgen from guest to host shows ~11% improvement for rx pps of tap:
>>>>>>
>>>>>> Before: ~1.70Mpps
>>>>>> After : ~1.88Mpps
>>>>>>
>>>>>> What's more important, this makes it possible to implement XDP
>>>>>> for tap
>>>>>> before creating skbs.
>>>>> Well well well.
>>>>>
>>>>> You do realize that tun_build_skb() is not thread safe ?
>>>> The issue is alloc frag, isn't it?
>>>> I guess for now we can limit this to XDP mode only, and
>>>> just allocate full pages in that mode.
>>>>
>>>>
>>> Limit this to XDP mode only does not prevent user from sending
>>> packets to
>>> same queue in parallel I think?
>>>
>>> Thanks
>> Yes but then you can just drop the page frag allocator since
>> XDP is assumed not to care about truesize for most packets.
>>
>
> Ok, let me do some test to see the numbers between the two methods first.
>
> Thanks
It looks like full page allocation just produce too much stress on the
page allocator.
I get 1.58Mpps (full page) vs 1.95Mpps (page frag) with the patches
attached.
Since non-XDP case can also benefit from build_skb(), I tend to use
spinlock instead of full page in this case.
Thanks
[-- Attachment #2: 0001-tun-thread-safe-tun_build_skb.patch --]
[-- Type: text/x-patch, Size: 3236 bytes --]
>From 0b9d930e8192466a9c4b85d136193f9c5f01d96a Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 16 Aug 2017 13:48:11 +0800
Subject: [PATCH] tun: thread safe tun_build_skb()
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/tun.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5892284..c72c2ea 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1247,6 +1247,8 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
static bool tun_can_build_skb(struct tun_struct *tun, struct tun_file *tfile,
int len, int noblock, bool zerocopy)
{
+ struct bpf_prog *xdp_prog;
+
if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
return false;
@@ -1263,7 +1265,11 @@ static bool tun_can_build_skb(struct tun_struct *tun, struct tun_file *tfile,
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) > PAGE_SIZE)
return false;
- return true;
+ rcu_read_lock();
+ xdp_prog = rcu_dereference(tun->xdp_prog);
+ rcu_read_unlock();
+
+ return xdp_prog;
}
static struct sk_buff *tun_build_skb(struct tun_struct *tun,
@@ -1272,7 +1278,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
struct virtio_net_hdr *hdr,
int len, int *generic_xdp)
{
- struct page_frag *alloc_frag = &tfile->alloc_frag;
+ struct page *page = alloc_page(GFP_KERNEL);
struct sk_buff *skb;
struct bpf_prog *xdp_prog;
int buflen = SKB_DATA_ALIGN(len + TUN_RX_PAD) +
@@ -1283,15 +1289,15 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
bool xdp_xmit = false;
int err;
- if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, GFP_KERNEL)))
+ if (unlikely(!page))
return ERR_PTR(-ENOMEM);
- buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
- copied = copy_page_from_iter(alloc_frag->page,
- alloc_frag->offset + TUN_RX_PAD,
- len, from);
- if (copied != len)
+ buf = (char *)page_address(page);
+ copied = copy_page_from_iter(page, TUN_RX_PAD, len, from);
+ if (copied != len) {
+ put_page(page);
return ERR_PTR(-EFAULT);
+ }
if (hdr->gso_type)
*generic_xdp = 1;
@@ -1313,11 +1319,9 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
switch (act) {
case XDP_REDIRECT:
- get_page(alloc_frag->page);
- alloc_frag->offset += buflen;
err = xdp_do_redirect(tun->dev, &xdp, xdp_prog);
if (err)
- goto err_redirect;
+ goto err_xdp;
return NULL;
case XDP_TX:
xdp_xmit = true;
@@ -1339,13 +1343,13 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
skb = build_skb(buf, buflen);
if (!skb) {
rcu_read_unlock();
+ put_page(page);
return ERR_PTR(-ENOMEM);
}
skb_reserve(skb, TUN_RX_PAD - delta);
skb_put(skb, len + delta);
- get_page(alloc_frag->page);
- alloc_frag->offset += buflen;
+ get_page(page);
if (xdp_xmit) {
skb->dev = tun->dev;
@@ -1358,9 +1362,8 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
return skb;
-err_redirect:
- put_page(alloc_frag->page);
err_xdp:
+ put_page(page);
rcu_read_unlock();
this_cpu_inc(tun->pcpu_stats->rx_dropped);
return NULL;
--
2.7.4
[-- Attachment #3: 0001-tun-serialize-page-frag-allocation.patch --]
[-- Type: text/x-patch, Size: 3489 bytes --]
>From c2aa968d97f1ac892a93d14cbbf06dac0b91fb49 Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 16 Aug 2017 13:17:15 +0800
Subject: [PATCH] tun: serialize page frag allocation
tun_build_skb() is not thread safe since we don't serialize protect
page frag allocation. This will lead crash when multiple threads are
sending packet into same tap queue in parallel. Solve this by
introducing a spinlock and use it to protect the page frag allocation.
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/tun.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5892284..202b20d 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -176,6 +176,7 @@ struct tun_file {
struct tun_struct *detached;
struct skb_array tx_array;
struct page_frag alloc_frag;
+ spinlock_t lock;
};
struct tun_flow_entry {
@@ -1273,25 +1274,36 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
int len, int *generic_xdp)
{
struct page_frag *alloc_frag = &tfile->alloc_frag;
+ struct page *page;
struct sk_buff *skb;
struct bpf_prog *xdp_prog;
int buflen = SKB_DATA_ALIGN(len + TUN_RX_PAD) +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
unsigned int delta = 0;
+ size_t offset;
char *buf;
size_t copied;
bool xdp_xmit = false;
int err;
- if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, GFP_KERNEL)))
+ spin_lock(&tfile->lock);
+ if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, GFP_KERNEL))) {
+ spin_unlock(&tfile->lock);
return ERR_PTR(-ENOMEM);
+ }
+
+ page = alloc_frag->page;
+ offset = alloc_frag->offset;
+ get_page(page);
+ buf = (char *)page_address(page) + offset;
+ alloc_frag->offset += buflen;
+ spin_unlock(&tfile->lock);
- buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
- copied = copy_page_from_iter(alloc_frag->page,
- alloc_frag->offset + TUN_RX_PAD,
- len, from);
- if (copied != len)
+ copied = copy_page_from_iter(page, offset + TUN_RX_PAD, len, from);
+ if (copied != len) {
+ put_page(page);
return ERR_PTR(-EFAULT);
+ }
if (hdr->gso_type)
*generic_xdp = 1;
@@ -1313,11 +1325,9 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
switch (act) {
case XDP_REDIRECT:
- get_page(alloc_frag->page);
- alloc_frag->offset += buflen;
err = xdp_do_redirect(tun->dev, &xdp, xdp_prog);
if (err)
- goto err_redirect;
+ goto err_xdp;
return NULL;
case XDP_TX:
xdp_xmit = true;
@@ -1339,13 +1349,12 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
skb = build_skb(buf, buflen);
if (!skb) {
rcu_read_unlock();
+ put_page(page);
return ERR_PTR(-ENOMEM);
}
skb_reserve(skb, TUN_RX_PAD - delta);
skb_put(skb, len + delta);
- get_page(alloc_frag->page);
- alloc_frag->offset += buflen;
if (xdp_xmit) {
skb->dev = tun->dev;
@@ -1358,9 +1367,8 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
return skb;
-err_redirect:
- put_page(alloc_frag->page);
err_xdp:
+ put_page(page);
rcu_read_unlock();
this_cpu_inc(tun->pcpu_stats->rx_dropped);
return NULL;
@@ -2586,6 +2594,7 @@ static int tun_chr_open(struct inode *inode, struct file * file)
INIT_LIST_HEAD(&tfile->next);
sock_set_flag(&tfile->sk, SOCK_ZEROCOPY);
+ spin_lock_init(&tfile->lock);
return 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY
From: Corentin Labbe @ 2017-08-16 9:12 UTC (permalink / raw)
To: Florian Fainelli
Cc: Chen-Yu Tsai, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, Russell King, Maxime Ripard,
Giuseppe Cavallaro, alexandre.torgue-qxv4g6HH51o, netdev,
devicetree, linux-kernel, linux-arm-kernel, andrew-g2DYL2Zd6BY
In-Reply-To: <302496B2-46F1-456D-A9A0-8257B5582695-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, Aug 11, 2017 at 08:03:29AM -0700, Florian Fainelli wrote:
> On August 11, 2017 6:25:26 AM PDT, Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >On Fri, Aug 11, 2017 at 04:22:11PM +0800, Chen-Yu Tsai wrote:
> >> On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
> >> <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> > On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
> >> >> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
> >> >> <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> >> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
> >> >> >> Hi,
> >> >> >>
> >> >> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
> >> >> >> <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> >> >> > This patch add the new phy-is-integrated property to the
> >internal PHY
> >> >> >> > node.
> >> >> >> >
> >> >> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >> >> > ---
> >> >> >> > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> >> >> >> > 1 file changed, 1 insertion(+)
> >> >> >> >
> >> >> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > index 4b599b5d26f6..54fc24e4c569 100644
> >> >> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > @@ -425,6 +425,7 @@
> >> >> >> > reg = <1>;
> >> >> >> > clocks = <&ccu
> >CLK_BUS_EPHY>;
> >> >> >> > resets = <&ccu
> >RST_BUS_EPHY>;
> >> >> >> > + phy-is-integrated;
> >> >> >>
> >> >> >> You also need to "delete" this property at the board level for
> >> >> >> any board that has the external PHY at address <1>. Otherwise
> >> >> >> they will stop working. This is due to the internal and
> >external
> >> >> >> PHYs having the same path and node name in the device tree, so
> >> >> >> they are effectively the same node.
> >> >> >>
> >> >> >> ChenYu
> >> >> >>
> >> >> >
> >> >> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
> >> >>
> >> >> That is just the label. The label plays no part in device tree
> >merging. The path
> >> >>
> >> >> /soc/ethernet@1c30000/mdio/ethernet-phy@1
> >> >>
> >> >> is the same. You can look under
> >> >>
> >> >> /proc/device-tree/soc/ethernet@1c30000/mdio
> >> >>
> >> >> on the OrangePI Plus 2E or any other H3 board that uses an
> >> >> external PHY at address 1.
> >> >>
> >> >> ChenYu
> >> >
> >> > Since we get the phy node by phy-handle and not by path, I think
> >all should be good.
> >>
> >> You are not getting me. The fact that the two seemingly separate
> >> nodes are merged together means, whatever properties you put in
> >> the internal PHY node, also affect the external PHY node. Once
> >> compiled, they are the SAME node.
> >
> >Hello Rob, florian, mark
> >
> >Adding a delete property on all external ethernet-phy@1 is a bit
> >overkill, and I dont like the idea that nodes are merged.
>
> This is not exactly up to you that's just how DTC works.
>
> >What do you think about other possible solutions:
> >- Using integrated-phy@1 for the integrated PHY node name
>
> That might be okay although you are using now a seemingly non-standard unit name.
>
> >- Using a fake address like 31 (see patch below)
>
> You could also drop the address part in the unit name although we'd probably get a DTC warning for that.
>
> I suspect both of your solutions and what I mentioned above will be producing DTC warnings to some extent... Rob what do you think?
>
I think I found an easier solution, putting phy-is-integrated on board DT nodes only.
I will send an updated serie.
Regards
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] net/mlx4: fix spelling mistake: "availible" -> "available"
From: Colin King @ 2017-08-16 9:05 UTC (permalink / raw)
To: Tariq Toukan, netdev, linux-rdma; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Trivial fix to spelling mistakes in the mlx4 driver
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlx4/cmd.c | 16 ++++++++--------
drivers/net/ethernet/mellanox/mlx4/fw_qos.c | 6 +++---
drivers/net/ethernet/mellanox/mlx4/fw_qos.h | 10 +++++-----
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 674773b28b2e..6309389b09a7 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -1958,19 +1958,19 @@ static void mlx4_allocate_port_vpps(struct mlx4_dev *dev, int port)
int i;
int err;
int num_vfs;
- u16 availible_vpp;
+ u16 available_vpp;
u8 vpp_param[MLX4_NUM_UP];
struct mlx4_qos_manager *port_qos;
struct mlx4_priv *priv = mlx4_priv(dev);
- err = mlx4_ALLOCATE_VPP_get(dev, port, &availible_vpp, vpp_param);
+ err = mlx4_ALLOCATE_VPP_get(dev, port, &available_vpp, vpp_param);
if (err) {
- mlx4_info(dev, "Failed query availible VPPs\n");
+ mlx4_info(dev, "Failed query available VPPs\n");
return;
}
port_qos = &priv->mfunc.master.qos_ctl[port];
- num_vfs = (availible_vpp /
+ num_vfs = (available_vpp /
bitmap_weight(port_qos->priority_bm, MLX4_NUM_UP));
for (i = 0; i < MLX4_NUM_UP; i++) {
@@ -1985,14 +1985,14 @@ static void mlx4_allocate_port_vpps(struct mlx4_dev *dev, int port)
}
/* Query actual allocated VPP, just to make sure */
- err = mlx4_ALLOCATE_VPP_get(dev, port, &availible_vpp, vpp_param);
+ err = mlx4_ALLOCATE_VPP_get(dev, port, &available_vpp, vpp_param);
if (err) {
- mlx4_info(dev, "Failed query availible VPPs\n");
+ mlx4_info(dev, "Failed query available VPPs\n");
return;
}
port_qos->num_of_qos_vfs = num_vfs;
- mlx4_dbg(dev, "Port %d Availible VPPs %d\n", port, availible_vpp);
+ mlx4_dbg(dev, "Port %d Availible VPPs %d\n", port, available_vpp);
for (i = 0; i < MLX4_NUM_UP; i++)
mlx4_dbg(dev, "Port %d UP %d Allocated %d VPPs\n", port, i,
@@ -2891,7 +2891,7 @@ static int mlx4_set_vport_qos(struct mlx4_priv *priv, int slave, int port,
memset(vpp_qos, 0, sizeof(struct mlx4_vport_qos_param) * MLX4_NUM_UP);
if (slave > port_qos->num_of_qos_vfs) {
- mlx4_info(dev, "No availible VPP resources for this VF\n");
+ mlx4_info(dev, "No available VPP resources for this VF\n");
return -EINVAL;
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw_qos.c b/drivers/net/ethernet/mellanox/mlx4/fw_qos.c
index 8f2fde0487c4..3a09d7122d3b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw_qos.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw_qos.c
@@ -65,7 +65,7 @@ struct mlx4_set_port_scheduler_context {
/* Granular Qos (per VF) section */
struct mlx4_alloc_vpp_param {
- __be32 availible_vpp;
+ __be32 available_vpp;
__be32 vpp_p_up[MLX4_NUM_UP];
};
@@ -157,7 +157,7 @@ int mlx4_SET_PORT_SCHEDULER(struct mlx4_dev *dev, u8 port, u8 *tc_tx_bw,
EXPORT_SYMBOL(mlx4_SET_PORT_SCHEDULER);
int mlx4_ALLOCATE_VPP_get(struct mlx4_dev *dev, u8 port,
- u16 *availible_vpp, u8 *vpp_p_up)
+ u16 *available_vpp, u8 *vpp_p_up)
{
int i;
int err;
@@ -179,7 +179,7 @@ int mlx4_ALLOCATE_VPP_get(struct mlx4_dev *dev, u8 port,
goto out;
/* Total number of supported VPPs */
- *availible_vpp = (u16)be32_to_cpu(out_param->availible_vpp);
+ *available_vpp = (u16)be32_to_cpu(out_param->available_vpp);
for (i = 0; i < MLX4_NUM_UP; i++)
vpp_p_up[i] = (u8)be32_to_cpu(out_param->vpp_p_up[i]);
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw_qos.h b/drivers/net/ethernet/mellanox/mlx4/fw_qos.h
index ac1f331878e6..582997577a04 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw_qos.h
+++ b/drivers/net/ethernet/mellanox/mlx4/fw_qos.h
@@ -84,23 +84,23 @@ int mlx4_SET_PORT_PRIO2TC(struct mlx4_dev *dev, u8 port, u8 *prio2tc);
int mlx4_SET_PORT_SCHEDULER(struct mlx4_dev *dev, u8 port, u8 *tc_tx_bw,
u8 *pg, u16 *ratelimit);
/**
- * mlx4_ALLOCATE_VPP_get - Query port VPP availible resources and allocation.
- * Before distribution of VPPs to priorities, only availible_vpp is returned.
+ * mlx4_ALLOCATE_VPP_get - Query port VPP available resources and allocation.
+ * Before distribution of VPPs to priorities, only available_vpp is returned.
* After initialization it returns the distribution of VPPs among priorities.
*
* @dev: mlx4_dev.
* @port: Physical port number.
- * @availible_vpp: Pointer to variable where number of availible VPPs is stored
+ * @available_vpp: Pointer to variable where number of available VPPs is stored
* @vpp_p_up: Distribution of VPPs to priorities is stored in this array
*
* Returns 0 on success or a negative mlx4_core errno code.
**/
int mlx4_ALLOCATE_VPP_get(struct mlx4_dev *dev, u8 port,
- u16 *availible_vpp, u8 *vpp_p_up);
+ u16 *available_vpp, u8 *vpp_p_up);
/**
* mlx4_ALLOCATE_VPP_set - Distribution of VPPs among differnt priorities.
* The total number of VPPs assigned to all for a port must not exceed
- * the value reported by availible_vpp in mlx4_ALLOCATE_VPP_get.
+ * the value reported by available_vpp in mlx4_ALLOCATE_VPP_get.
* VPP allocation is allowed only after the port type has been set,
* and while no QPs are open for this port.
*
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy.
From: Jakub Sitnicki @ 2017-08-16 9:03 UTC (permalink / raw)
To: Jonathan Basseri; +Cc: netdev, davem, steffen.klassert, lorenzo
In-Reply-To: <20170815222510.21711-1-misterikkit@google.com>
On Tue, 15 Aug 2017 15:25:10 -0700
Jonathan Basseri <misterikkit@google.com> wrote:
> If an IPv6 socket has a valid dst cache, then xfrm_lookup_route will get
> skipped. However, the cache is not invalidated when applying policy to a
> socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
> sometimes ignored on those sockets.
>
> This can be demonstrated like so,
> 1. Create UDPv6 socket.
> 2. connect() the socket.
> 3. Apply an outbound XFRM policy to the socket.
> 4. send() data on the socket.
>
> Packets will continue to be sent in the clear instead of matching an
> xfrm or returning a no-match error (EAGAIN). This affects calls to
> send() and not sendto().
>
> Note: Creating normal XFRM policies should have a similar effect on
> sk_dst_cache entries that match the policy, but that is not fixed in
> this patch.
>
> Fixes: 00bc0ef5880d ("ipv6: Skip XFRM lookup if dst_entry in socket cache is valid")
> Tested: https://android-review.googlesource.com/418659
> Signed-off-by: Jonathan Basseri <misterikkit@google.com>
> ---
Thank you for the fix.
Acked-by: Jakub Sitnicki <jkbs@redhat.com>
^ permalink raw reply
* Re: [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy.
From: Lorenzo Colitti @ 2017-08-16 9:03 UTC (permalink / raw)
To: Jonathan Basseri
Cc: netdev@vger.kernel.org, jkbs, David Miller, Steffen Klassert
In-Reply-To: <20170815222510.21711-1-misterikkit@google.com>
On Wed, Aug 16, 2017 at 7:25 AM, Jonathan Basseri
<misterikkit@google.com> wrote:
> If an IPv6 socket has a valid dst cache
Did you look into why IPv4 does not suffer from this problem?
That said, clearing the dst cache entry does seem prudent in general.
^ permalink raw reply
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
From: Christian König @ 2017-08-16 8:55 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Chris Mi, netdev
In-Reply-To: <20170816083939.GD1868@nanopsycho>
Am 16.08.2017 um 10:39 schrieb Jiri Pirko:
> Wed, Aug 16, 2017 at 10:31:35AM CEST, christian.koenig@amd.com wrote:
>> Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
>>> Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
>>>> Am 16.08.2017 um 04:12 schrieb Chris Mi:
>>>>> Using current TC code, it is very slow to insert a lot of rules.
>>>>>
>>>>> In order to improve the rules update rate in TC,
>>>>> we introduced the following two changes:
>>>>> 1) changed cls_flower to use IDR to manage the filters.
>>>>> 2) changed all act_xxx modules to use IDR instead of
>>>>> a small hash table
>>>>>
>>>>> But IDR has a limitation that it uses int. TC handle uses u32.
>>>>> To make sure there is no regression, we also changed IDR to use
>>>>> unsigned long. All clients of IDR are changed to use new IDR API.
>>>> WOW, wait a second. The idr change is touching a lot of drivers and to be
>>>> honest doesn't looks correct at all.
>>>>
>>>> Just look at the first chunk of your modification:
>>>>> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>>>>> mutex_lock(&bsg_mutex);
>>>>> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
>>>>> - if (ret < 0) {
>>>>> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
>>>>> + GFP_KERNEL);
>>>>> + if (ret) {
>>>>> if (ret == -ENOSPC) {
>>>>> printk(KERN_ERR "bsg: too many bsg devices\n");
>>>>> ret = -EINVAL;
>>>> The condition "if (ret)" will now always be true after the first allocation
>>>> and so we always run into the error handling after that.
>>> On success, idr_alloc returns 0.
>> Ah, I see. You change the idr_alloc to return the resulting index as separate
>> parameter.
>>
>> You should explicit note that in the commit message, cause that is something
>> easily overlooked.
>>
>> In general I strongly suggest to add a separate interface for allocating
>> unsigned long handles, use that for the while being and then move the
>> existing drivers over bit by bit.
>>
>> A single patch which touches so many different driver is practically
>> impossible to review consequently.
> Understood. I think is is good to avoid having some "idr_alloc2". That
> is why I suggested to do this in one go, to avoid "idr_alloc2" and then
> patch to rename "idr_alloc2" to "idr_alloc" once nobody uses the original
> "idr_alloc". In fact, if you do it driver, by driver, the review burden
> would be the same, probably even bigger, you'll just have 100+ patches.
> Why would it help?
Because it would give each maintainer only the part of the change he is
interested in.
Current status of this change is that you send a mail with nearly 300
people on CC.
Do you really expect to get an reviewed-by or acked-by on this single
patch from all of them?
If yes then it somehow makes sense to send the patch bit by bit, if no
then it doesn't seem to make to much sense to CC them all individually.
>>>> I've never read the bsg code before, but that's certainly not correct. And
>>>> that incorrect pattern repeats over and over again in this code.
>>>>
>>>> Apart from that why the heck do you want to allocate more than 1<<31 handles?
>>> tc action indexes for example. That is part of this patchset.
>> Well, let me refine the question: Why does tc action indexes need more than
>> 31 bits? From an outside view that looks like pure overkill.
> That is current state, uapi. We have to live with it.
Is the range to allocate from part of the uapi or what is the issue here?
If the issue is that userspace can specify the handle then I suggest
that you use the radix tree directly instead of the idr wrapper around it.
Regards,
Christian.
^ permalink raw reply
* Re: [net-next PATCH] bpf: devmap: remove unnecessary value size check
From: Daniel Borkmann @ 2017-08-16 8:39 UTC (permalink / raw)
To: John Fastabend, davem; +Cc: netdev
In-Reply-To: <20170816063512.14925.40390.stgit@john-Precision-Tower-5810>
On 08/16/2017 08:35 AM, John Fastabend wrote:
> In the devmap alloc map logic we check to ensure that the sizeof the
> values are not greater than KMALLOC_MAX_SIZE. But, in the dev map case
> we ensure the value size is 4bytes earlier in the function because all
> values should be netdev ifindex values.
>
> The second check is harmless but is not needed so remove it.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net] net: sched: fix NULL pointer dereference when action calls some targets
From: Xin Long @ 2017-08-16 8:39 UTC (permalink / raw)
To: Cong Wang; +Cc: network dev, David Miller, netfilter-devel, Jamal Hadi Salim
In-Reply-To: <CAM_iQpXbtHDxVjZ=DoTiLXHLQy6q=JVBa66EqSOy95ZbFg-wkQ@mail.gmail.com>
On Wed, Aug 9, 2017 at 7:33 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Aug 7, 2017 at 7:33 PM, Xin Long <lucien.xin@gmail.com> wrote:
>> On Tue, Aug 8, 2017 at 9:15 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> This looks like a completely API burden?
>> netfilter xt targets are not really compatible with netsched action.
>> I've got to say, the patch is just a way to make checkentry return
>> false and avoid panic. like [1] said
>
> I don't doubt you fix a crash, I am thinking if we can
> "fix" the API instead of fixing the caller.
Hi, Cong,
For now, I don't think it's possible to change APIs or some of their targets
for the panic caused by action xt calling.
The common way should be fixed in net_sched side.
Given that the issue is very easy to triggered,
let's wait for netfilter's replies for another few days,
otherwise I will repost the fix, agree ?
>
> I am not familiar with this API, so just my 2 cents...
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox