* Re: Kernel 4.13.0-rc4-next-20170811 - IP Routing / Forwarding performance vs Core/RSS number / HT on
From: Paweł Staszewski @ 2017-09-11 22:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paolo Abeni, Jesper Dangaard Brouer,
Linux Kernel Network Developers, Alexander Duyck
In-Reply-To: <0123de5c-a7ad-7e62-b7d0-cd19bd34c573@itcare.pl>
Another test for this patch with linux-next tree
with patch:
bwm-ng v0.6.1 (probing every 1.000s), press 'h' for help
input: /proc/net/dev type: rate
- iface Rx Tx Total
==============================================================================
vlan1004: 1.00 P/s 606842.31 P/s 606843.31 P/s
lo: 0.00 P/s 0.00 P/s
0.00 P/s
vlan1016: 0.00 P/s 607730.56 P/s 607730.56 P/s
vlan1020: 0.00 P/s 606891.25 P/s 606891.25 P/s
vlan1018: 0.00 P/s 607580.88 P/s 607580.88 P/s
vlan1014: 0.00 P/s 607606.81 P/s 607606.81 P/s
vlan1005: 0.00 P/s 606788.44 P/s 606788.44 P/s
enp2s0f0: 2.00 P/s 2.00 P/s
3.99 P/s
vlan1017: 0.00 P/s 607643.75 P/s 607643.75 P/s
enp132s0: 13079658.00 P/s 0.00 P/s 13079658.00 P/s
vlan1000: 0.00 P/s 604409.19 P/s 604409.19 P/s
vlan1010: 0.00 P/s 606984.06 P/s 606984.06 P/s
vlan1019: 0.00 P/s 607452.12 P/s 607452.12 P/s
vlan1008: 0.00 P/s 606803.44 P/s 606803.44 P/s
vlan1011: 0.00 P/s 607048.94 P/s 607048.94 P/s
vlan1001: 0.00 P/s 606773.50 P/s 606773.50 P/s
vlan1006: 0.00 P/s 606811.38 P/s 606811.38 P/s
vlan1012: 0.00 P/s 607051.94 P/s 607051.94 P/s
vlan1013: 0.00 P/s 607067.88 P/s 607067.88 P/s
enp4s0: 2.00 P/s 13020803.00 P/s 13020805.00 P/s
vlan1007: 0.00 P/s 606798.44 P/s 606798.44 P/s
vlan1002: 0.00 P/s 606840.31 P/s 606840.31 P/s
vlan1009: 0.00 P/s 606809.38 P/s 606809.38 P/s
enp2s0f1: 100.80 P/s 0.00 P/s
100.80 P/s
vlan1015: 0.00 P/s 607089.81 P/s 607089.81 P/s
vlan1003: 1.00 P/s 606928.19 P/s 606929.19 P/s
------------------------------------------------------------------------------
total: 13079765.00 P/s 25766758.00 P/s 38846524.00 P/s
13Mpps forwarded (32cores active for two mlx5 nics)
80% cpu load (20%idle all cores)
PerfTop: 126552 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
cycles], (all, 32 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
8.25% [kernel] [k] fib_table_lookup
7.98% [kernel] [k] do_raw_spin_lock
6.20% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
4.21% [kernel] [k] mlx5e_xmit
3.37% [kernel] [k] __dev_queue_xmit
2.95% [kernel] [k] ip_rcv
2.72% [kernel] [k] ipt_do_table
2.24% [kernel] [k] ip_finish_output2
2.22% [kernel] [k] __netif_receive_skb_core
2.17% [kernel] [k] ip_forward
2.15% [kernel] [k] __build_skb
1.99% [kernel] [k] ip_route_input_rcu
1.70% [kernel] [k] mlx5e_txwqe_complete
1.54% [kernel] [k] dev_gro_receive
1.45% [kernel] [k] mlx5_cqwq_get_cqe
1.38% [kernel] [k] udp_v4_early_demux
1.35% [kernel] [k] netif_skb_features
1.33% [kernel] [k] inet_gro_receive
1.29% [kernel] [k] dev_hard_start_xmit
1.27% [kernel] [k] ip_rcv_finish
1.19% [kernel] [k] mlx5e_build_rx_skb
1.15% [kernel] [k] __netdev_pick_tx
1.11% [kernel] [k] kmem_cache_alloc
1.09% [kernel] [k] mlx5e_poll_tx_cq
1.07% [kernel] [k] mlx5e_txwqe_build_dsegs
1.00% [kernel] [k] vlan_dev_hard_start_xmit
0.90% [kernel] [k] __napi_alloc_skb
0.87% [kernel] [k] validate_xmit_skb
0.87% [kernel] [k] read_tsc
0.83% [kernel] [k] napi_gro_receive
0.79% [kernel] [k] skb_network_protocol
0.79% [kernel] [k] sch_direct_xmit
0.78% [kernel] [k] __local_bh_enable_ip
0.78% [kernel] [k] netdev_pick_tx
0.75% [kernel] [k] __udp4_lib_lookup
0.72% [kernel] [k] netif_receive_skb_internal
0.71% [kernel] [k] page_frag_free
0.71% [kernel] [k] deliver_ptype_list_skb
0.70% [kernel] [k] fib_validate_source
0.69% [kernel] [k] mlx5_cqwq_get_cqe
0.69% [kernel] [k] __netif_receive_skb
0.68% [kernel] [k] vlan_passthru_hard_header
0.61% [kernel] [k] rt_cache_valid
0.59% [kernel] [k] iptable_filter_hook
Without patch:
12,7Mpps forwarded (32cores active for two mlx5 nics)
100% cpu load ( rx drops on receiving side )
TX is about 13.05Mpps from pktgen
bwm-ng v0.6.1 (probing every 1.000s), press 'h' for help
input: /proc/net/dev type: rate
/ iface Rx Tx Total
==============================================================================
vlan1004: 0.00 P/s 589709.31 P/s 589709.31 P/s
lo: 0.00 P/s 0.00 P/s
0.00 P/s
vlan1016: 0.00 P/s 589495.50 P/s 589495.50 P/s
vlan1020: 0.00 P/s 589968.06 P/s 589968.06 P/s
vlan1018: 0.00 P/s 589896.12 P/s 589896.12 P/s
vlan1014: 0.00 P/s 589496.50 P/s 589496.50 P/s
vlan1005: 0.00 P/s 589502.50 P/s 589502.50 P/s
enp2s0f0: 42.96 P/s 2.00 P/s
44.96 P/s
vlan1017: 0.00 P/s 589508.50 P/s 589508.50 P/s
enp132s0: 12700689.00 P/s 0.00 P/s 12700689.00 P/s
vlan1000: 0.00 P/s 587671.38 P/s 587671.38 P/s
vlan1010: 0.00 P/s 589330.69 P/s 589330.69 P/s
vlan1019: 0.00 P/s 589808.19 P/s 589808.19 P/s
vlan1008: 0.00 P/s 589283.75 P/s 589283.75 P/s
vlan1011: 0.00 P/s 589482.56 P/s 589482.56 P/s
vlan1001: 0.00 P/s 589971.06 P/s 589971.06 P/s
vlan1006: 0.00 P/s 589785.25 P/s 589785.25 P/s
vlan1012: 0.00 P/s 589494.50 P/s 589494.50 P/s
vlan1013: 0.00 P/s 589495.50 P/s 589495.50 P/s
enp4s0: 0.00 P/s 12601841.00 P/s 12601841.00 P/s
vlan1007: 0.00 P/s 589537.50 P/s 589537.50 P/s
vlan1002: 0.00 P/s 589943.06 P/s 589943.06 P/s
vlan1009: 0.00 P/s 589306.69 P/s 589306.69 P/s
enp2s0f1: 99.90 P/s 0.00 P/s
99.90 P/s
vlan1015: 0.00 P/s 589553.44 P/s 589553.44 P/s
vlan1003: 0.00 P/s 589823.19 P/s 589823.19 P/s
------------------------------------------------------------------------------
total: 12700832.00 P/s 24981906.00 P/s 37682740.00 P/s
PerfTop: 124056 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
cycles], (all, 32 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
7.77% [kernel] [k] fib_table_lookup
7.37% [kernel] [k] do_raw_spin_lock
5.43% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
3.84% [kernel] [k] mlx5e_xmit
3.52% [kernel] [k] rt_cache_valid
3.46% [kernel] [k] ip_finish_output2
3.45% [kernel] [k] skb_dst_force
2.96% [kernel] [k] dst_release
2.62% [kernel] [k] __dev_queue_xmit
2.54% [kernel] [k] ip_rcv
2.26% [kernel] [k] ipt_do_table
1.97% [kernel] [k] __build_skb
1.94% [kernel] [k] __netif_receive_skb_core
1.94% [kernel] [k] ip_route_input_rcu
1.83% [kernel] [k] ip_forward
1.52% [kernel] [k] mlx5e_txwqe_complete
1.42% [kernel] [k] dev_gro_receive
1.31% [kernel] [k] mlx5_cqwq_get_cqe
1.28% [kernel] [k] netif_skb_features
1.21% [kernel] [k] ip_rcv_finish
1.18% [kernel] [k] inet_gro_receive
1.17% [kernel] [k] udp_v4_early_demux
1.14% [kernel] [k] dev_hard_start_xmit
1.03% [kernel] [k] mlx5e_txwqe_build_dsegs
1.02% [kernel] [k] mlx5e_poll_tx_cq
1.02% [kernel] [k] mlx5e_build_rx_skb
1.00% [kernel] [k] kmem_cache_alloc
0.95% [kernel] [k] __netdev_pick_tx
0.93% [kernel] [k] vlan_dev_hard_start_xmit
0.82% [kernel] [k] __napi_alloc_skb
0.79% [kernel] [k] read_tsc
0.76% [kernel] [k] validate_xmit_skb
0.75% [kernel] [k] napi_gro_receive
0.71% [kernel] [k] __local_bh_enable_ip
0.70% [kernel] [k] sch_direct_xmit
0.67% [kernel] [k] page_frag_free
0.66% [kernel] [k] skb_network_protocol
0.66% [kernel] [k] nf_hook_slow
0.63% [kernel] [k] netif_receive_skb_internal
0.62% [kernel] [k] vlan_passthru_hard_header
0.62% [kernel] [k] deliver_ptype_list_skb
0.62% [kernel] [k] __netif_receive_skb
0.61% [kernel] [k] fib_validate_source
0.61% [kernel] [k] mlx5_cqwq_get_cqe
0.55% [kernel] [k] eth_type_trans
0.55% [kernel] [k] iptable_filter_hook
0.53% [kernel] [k] __udp4_lib_lookup
0.53% [kernel] [k] udp_gro_receive
0.51% [kernel] [k] eth_header
0.49% [kernel] [k] fib_lookup.constprop.49
0.48% [kernel] [k] ip_output
0.44% [kernel] [k] netdev_pick_tx
0.43% [kernel] [k] validate_xmit_skb_list
0.42% [kernel] [k] swiotlb_map_page
0.41% [kernel] [k] dma_sync_single_for_cpu.constprop.36
0.41% [kernel] [k] mlx5_cqwq_get_wqe
0.39% [kernel] [k] eth_type_vlan
0.38% [kernel] [k] udp4_gro_receive
0.37% [kernel] [k] __jhash_nwords
0.37% [kernel] [k] ip_forward_finish
0.33% [kernel] [k] ip_finish_output
0.33% [kernel] [k] neigh_connected_output
0.31% [kernel] [k] mlx5e_features_check
0.29% [kernel] [k] ktime_get_with_offset
0.27% [kernel] [k] __udp4_lib_lookup_skb
0.27% [kernel] [k] napi_consume_skb
0.26% [kernel] [k] page_frag_alloc
0.26% [kernel] [k] get_dma_ops
0.25% [kernel] [k] udp4_portaddr_hash.isra.29
0.24% [kernel] [k] skb_dst_drop.isra.76
0.24% [kernel] [k] neigh_resolve_output
0.24% [kernel] [k] skb_release_data
0.22% [kernel] [k] kmem_cache_free_bulk
0.22% [kernel] [k] get_dma_ops
0.22% [kernel] [k] _kfree_skb_defer
0.21% [kernel] [k] ip_skb_dst_mtu
0.21% [kernel] [k] is_swiotlb_buffer
0.21% [kernel] [k] find_exception
0.21% [kernel] [k] compound_head
0.21% [kernel] [k] __net_timestamp.isra.89
W dniu 2017-09-11 o 18:57, Paweł Staszewski pisze:
> Tested with connectx-5
>
> Without patch
>
> 10Mpps - > 16 cores used
>
> PerfTop: 66258 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
> cycles], (all, 32 CPUs)
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>
> 10.12% [kernel] [k] do_raw_spin_lock
> 6.31% [kernel] [k] fib_table_lookup
> 6.12% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
> 4.90% [kernel] [k] rt_cache_valid
> 3.99% [kernel] [k] mlx5e_xmit
> 3.03% [kernel] [k] ip_rcv
> 2.68% [kernel] [k] __netif_receive_skb_core
> 2.54% [kernel] [k] skb_dst_force
> 2.41% [kernel] [k] ip_finish_output2
> 2.21% [kernel] [k] __build_skb
> 2.03% [kernel] [k] __dev_queue_xmit
> 1.96% [kernel] [k] mlx5e_txwqe_complete
> 1.79% [kernel] [k] ipt_do_table
> 1.78% [kernel] [k] inet_gro_receive
> 1.69% [kernel] [k] ip_forward
> 1.66% [kernel] [k] udp_v4_early_demux
> 1.65% [kernel] [k] dst_release
> 1.56% [kernel] [k] ip_rcv_finish
> 1.45% [kernel] [k] dev_gro_receive
> 1.45% [kernel] [k] netif_skb_features
> 1.39% [kernel] [k] mlx5e_poll_tx_cq
> 1.35% [kernel] [k] mlx5e_txwqe_build_dsegs
> 1.35% [kernel] [k] ip_route_input_rcu
> 1.15% [kernel] [k] dev_hard_start_xmit
> 1.12% [kernel] [k] napi_gro_receive
> 1.07% [kernel] [k] netif_receive_skb_internal
> 0.98% [kernel] [k] sch_direct_xmit
> 0.95% [kernel] [k] kmem_cache_alloc
> 0.89% [kernel] [k] read_tsc
> 0.88% [kernel] [k] mlx5e_build_rx_skb
> 0.86% [kernel] [k] mlx5_cqwq_get_cqe
> 0.82% [kernel] [k] page_frag_free
> 0.78% [kernel] [k] __local_bh_enable_ip
> 0.69% [kernel] [k] skb_network_protocol
> 0.68% [kernel] [k] __netif_receive_skb
> 0.67% [kernel] [k] vlan_dev_hard_start_xmit
> 0.65% [kernel] [k] mlx5e_poll_rx_cq
> 0.65% [kernel] [k] validate_xmit_skb
> 0.60% [kernel] [k] eth_type_trans
> 0.60% [kernel] [k] deliver_ptype_list_skb
> 0.60% [kernel] [k] fib_validate_source
> 0.55% [kernel] [k] eth_header
> 0.53% [kernel] [k] netdev_pick_tx
> 0.53% [kernel] [k] __napi_alloc_skb
> 0.51% [kernel] [k] __udp4_lib_lookup
> 0.50% [kernel] [k] eth_type_vlan
> 0.49% [kernel] [k] ip_output
> 0.49% [kernel] [k] page_frag_alloc
> 0.49% [kernel] [k] ip_finish_output
> 0.48% [kernel] [k] neigh_connected_output
> 0.45% [kernel] [k] nf_hook_slow
> 0.44% [kernel] [k] udp4_gro_receive
> 0.39% [kernel] [k] mlx5e_features_check
> 0.39% [kernel] [k] mlx5e_napi_poll
> 0.37% [kernel] [k] __jhash_nwords
> 0.37% [kernel] [k] udp_gro_receive
> 0.36% [kernel] [k] swiotlb_map_page
> 0.33% [kernel] [k] mlx5_cqwq_get_wqe
> 0.33% [kernel] [k] __netdev_pick_tx
> 0.29% [kernel] [k] ktime_get_with_offset
> 0.29% [kernel] [k] get_dma_ops
> 0.29% [kernel] [k] validate_xmit_skb_list
> 0.26% [kernel] [k] vlan_passthru_hard_header
> 0.26% [kernel] [k] __udp4_lib_lookup_skb
> 0.24% [kernel] [k] get_dma_ops
> 0.24% [kernel] [k] skb_release_data
> 0.23% [kernel] [k] ip_forward_finish
> 0.23% [kernel] [k] kmem_cache_free_bulk
> 0.23% [kernel] [k] timekeeping_get_ns
> 0.22% [kernel] [k] ip_skb_dst_mtu
> 0.21% [kernel] [k] compound_head
> 0.20% [kernel] [k] skb_gro_reset_offset
> 0.20% [kernel] [k] is_swiotlb_buffer
> 0.19% [kernel] [k] __net_timestamp.isra.90
> 0.19% [kernel] [k] dst_metric.constprop.61
> 0.18% [kernel] [k] skb_orphan_frags.constprop.126
> 0.18% [kernel] [k] _kfree_skb_defer
> 0.18% [kernel] [k] irq_entries_start
> 0.17% [kernel] [k] dev_hard_header.constprop.54
> 0.17% [kernel] [k] dma_mapping_error
> 0.17% [kernel] [k] neigh_resolve_output
>
>
>
>
> With patch
>
> 12Mpps -> 16 cores
>
> PerfTop: 66209 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
> cycles], (all, 32 CPUs)
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>
> 10.67% [kernel] [k] do_raw_spin_lock
> 6.96% [kernel] [k] fib_table_lookup
> 6.53% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
> 4.17% [kernel] [k] mlx5e_xmit
> 3.22% [kernel] [k] ip_rcv
> 3.07% [kernel] [k] __netif_receive_skb_core
> 2.86% [kernel] [k] __dev_queue_xmit
> 2.36% [kernel] [k] __build_skb
> 2.33% [kernel] [k] ip_forward
> 2.05% [kernel] [k] mlx5e_txwqe_complete
> 2.02% [kernel] [k] ip_finish_output2
> 2.00% [kernel] [k] ipt_do_table
> 1.84% [kernel] [k] ip_rcv_finish
> 1.83% [kernel] [k] inet_gro_receive
> 1.80% [kernel] [k] udp_v4_early_demux
> 1.61% [kernel] [k] dev_gro_receive
> 1.55% [kernel] [k] netif_skb_features
> 1.52% [kernel] [k] mlx5e_txwqe_build_dsegs
> 1.47% [kernel] [k] mlx5e_poll_tx_cq
> 1.39% [kernel] [k] ip_route_input_rcu
> 1.38% [kernel] [k] dev_hard_start_xmit
> 1.17% [kernel] [k] netif_receive_skb_internal
> 1.16% [kernel] [k] napi_gro_receive
> 1.03% [kernel] [k] kmem_cache_alloc
> 1.02% [kernel] [k] sch_direct_xmit
> 0.97% [kernel] [k] read_tsc
> 0.94% [kernel] [k] page_frag_free
> 0.91% [kernel] [k] mlx5_cqwq_get_cqe
> 0.90% [kernel] [k] mlx5e_build_rx_skb
> 0.89% [kernel] [k] skb_network_protocol
> 0.83% [kernel] [k] __local_bh_enable_ip
> 0.79% [kernel] [k] validate_xmit_skb
> 0.77% [kernel] [k] vlan_dev_hard_start_xmit
> 0.74% [kernel] [k] __netif_receive_skb
> 0.72% [kernel] [k] mlx5e_poll_rx_cq
> 0.70% [kernel] [k] netdev_pick_tx
> 0.69% [kernel] [k] eth_type_vlan
> 0.68% [kernel] [k] __netdev_pick_tx
> 0.66% [kernel] [k] nf_hook_slow
> 0.65% [kernel] [k] deliver_ptype_list_skb
> 0.62% [kernel] [k] fib_validate_source
> 0.61% [kernel] [k] eth_header
> 0.60% [kernel] [k] eth_type_trans
> 0.59% [kernel] [k] __udp4_lib_lookup
> 0.58% [kernel] [k] __napi_alloc_skb
> 0.53% [kernel] [k] ip_finish_output
> 0.51% [kernel] [k] neigh_connected_output
> 0.50% [kernel] [k] ip_output
> 0.50% [kernel] [k] rt_cache_valid
> 0.44% [kernel] [k] udp4_gro_receive
> 0.43% [kernel] [k] mlx5e_napi_poll
> 0.40% [kernel] [k] udp_gro_receive
> 0.40% [kernel] [k] page_frag_alloc
> 0.40% [kernel] [k] __jhash_nwords
> 0.39% [kernel] [k] swiotlb_map_page
> 0.38% [kernel] [k] mlx5_cqwq_get_wqe
> 0.36% [kernel] [k] mlx5e_features_check
> 0.32% [kernel] [k] get_dma_ops
> 0.31% [kernel] [k] ktime_get_with_offset
> 0.31% [kernel] [k] validate_xmit_skb_list
> 0.28% [kernel] [k] vlan_passthru_hard_header
> 0.28% [kernel] [k] get_dma_ops
> 0.27% [kernel] [k] __udp4_lib_lookup_skb
> 0.26% [kernel] [k] skb_gro_reset_offset
> 0.25% [kernel] [k] skb_release_data
> 0.25% [kernel] [k] timekeeping_get_ns
> 0.24% [kernel] [k] kmem_cache_free_bulk
> 0.24% [kernel] [k] ip_forward_finish
> 0.23% [kernel] [k] compound_head
> 0.23% [kernel] [k] ip_skb_dst_mtu
> 0.22% [kernel] [k] __net_timestamp.isra.90
> 0.22% [kernel] [k] is_swiotlb_buffer
> 0.21% [kernel] [k] neigh_resolve_output
> 0.21% [kernel] [k] dst_metric.constprop.61
> 0.20% [kernel] [k] skb_orphan_frags.constprop.126
> 0.20% [kernel] [k] irq_entries_start
> 0.19% [kernel] [k] mlx5e_calc_min_inline
> 0.19% [kernel] [k] dev_hard_header.constprop.54
> 0.19% [kernel] [k] _kfree_skb_defer
> 0.18% [kernel] [k] _raw_spin_lock
> 0.18% [kernel] [k] ip_route_input_noref
>
>
>
>
>
> W dniu 2017-09-09 o 11:03, Paweł Staszewski pisze:
>> Hi
>>
>>
>> Are there any plans to have this fix normally in kernel ?
>>
>> Or it is mostly only hack - not longterm fix and need to be different ?
>>
>>
>> All tests that was done shows that without this patch there is about
>> 20-30% network forwarding performance degradation when using vlan
>> interfaces
>>
>>
>> Thanks
>> Paweł
>>
>>
>>
>> W dniu 2017-08-15 o 03:17, Eric Dumazet pisze:
>>> On Mon, 2017-08-14 at 18:07 -0700, Eric Dumazet wrote:
>>>
>>>> Or try to hack the IFF_XMIT_DST_RELEASE flag on the vlan netdev.
>>> Something like :
>>>
>>> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
>>> index
>>> 5e831de3103e2f7092c7fa15534def403bc62fb4..9472de846d5c0960996261cb2843032847fa4bf7
>>> 100644
>>> --- a/net/8021q/vlan_netlink.c
>>> +++ b/net/8021q/vlan_netlink.c
>>> @@ -143,6 +143,7 @@ static int vlan_newlink(struct net *src_net,
>>> struct net_device *dev,
>>> vlan->vlan_proto = proto;
>>> vlan->vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
>>> vlan->real_dev = real_dev;
>>> + dev->priv_flags |= (real_dev->priv_flags & IFF_XMIT_DST_RELEASE);
>>> vlan->flags = VLAN_FLAG_REORDER_HDR;
>>> err = vlan_check_real_dev(real_dev, vlan->vlan_proto,
>>> vlan->vlan_id);
>>>
>>>
>>>
>>>
>>
>>
>
>
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Tom Herbert @ 2017-09-11 22:07 UTC (permalink / raw)
To: Samudrala, Sridhar; +Cc: Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <f98198c4-16f2-5271-3e12-7984a03bd6db@intel.com>
On Mon, Sep 11, 2017 at 9:49 AM, Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
> On 9/10/2017 8:19 AM, Tom Herbert wrote:
>>
>> On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
>> <sridhar.samudrala@intel.com> wrote:
>>>
>>> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be
>>> used
>>> to enable symmetric tx and rx queues on a socket.
>>>
>>> This option is specifically useful for epoll based multi threaded
>>> workloads
>>> where each thread handles packets received on a single RX queue . In this
>>> model,
>>> we have noticed that it helps to send the packets on the same TX queue
>>> corresponding to the queue-pair associated with the RX queue specifically
>>> when
>>> busy poll is enabled with epoll().
>>>
>>> Two new fields are added to struct sock_common to cache the last rx
>>> ifindex and
>>> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns
>>> the cached
>>> rx queue when this option is enabled and the TX is happening on the same
>>> device.
>>>
>>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>> ---
>>> include/net/request_sock.h | 1 +
>>> include/net/sock.h | 17 +++++++++++++++++
>>> include/uapi/asm-generic/socket.h | 2 ++
>>> net/core/dev.c | 8 +++++++-
>>> net/core/sock.c | 10 ++++++++++
>>> net/ipv4/tcp_input.c | 1 +
>>> net/ipv4/tcp_ipv4.c | 1 +
>>> net/ipv4/tcp_minisocks.c | 1 +
>>> 8 files changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
>>> index 23e2205..c3bc12e 100644
>>> --- a/include/net/request_sock.h
>>> +++ b/include/net/request_sock.h
>>> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct
>>> request_sock *req)
>>> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
>>> sk_node_init(&req_to_sk(req)->sk_node);
>>> sk_tx_queue_clear(req_to_sk(req));
>>> + req_to_sk(req)->sk_symmetric_queues =
>>> sk_listener->sk_symmetric_queues;
>>> req->saved_syn = NULL;
>>> refcount_set(&req->rsk_refcnt, 0);
>>>
>>> diff --git a/include/net/sock.h b/include/net/sock.h
>>> index 03a3625..3421809 100644
>>> --- a/include/net/sock.h
>>> +++ b/include/net/sock.h
>>> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char
>>> *msg, ...)
>>> * @skc_node: main hash linkage for various protocol lookup tables
>>> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
>>> * @skc_tx_queue_mapping: tx queue number for this connection
>>> + * @skc_rx_queue_mapping: rx queue number for this connection
>>> + * @skc_rx_ifindex: rx ifindex for this connection
>>> * @skc_flags: place holder for sk_flags
>>> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
>>> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
>>> * @skc_incoming_cpu: record/match cpu processing incoming packets
>>> * @skc_refcnt: reference count
>>> + * @skc_symmetric_queues: symmetric tx/rx queues
>>> *
>>> * This is the minimal network layer representation of sockets, the
>>> header
>>> * for struct sock and struct inet_timewait_sock.
>>> @@ -177,6 +180,7 @@ struct sock_common {
>>> unsigned char skc_reuseport:1;
>>> unsigned char skc_ipv6only:1;
>>> unsigned char skc_net_refcnt:1;
>>> + unsigned char skc_symmetric_queues:1;
>>> int skc_bound_dev_if;
>>> union {
>>> struct hlist_node skc_bind_node;
>>> @@ -214,6 +218,8 @@ struct sock_common {
>>> struct hlist_nulls_node skc_nulls_node;
>>> };
>>> int skc_tx_queue_mapping;
>>> + int skc_rx_queue_mapping;
>>> + int skc_rx_ifindex;
>>> union {
>>> int skc_incoming_cpu;
>>> u32 skc_rcv_wnd;
>>> @@ -324,6 +330,8 @@ struct sock {
>>> #define sk_nulls_node __sk_common.skc_nulls_node
>>> #define sk_refcnt __sk_common.skc_refcnt
>>> #define sk_tx_queue_mapping __sk_common.skc_tx_queue_mapping
>>> +#define sk_rx_queue_mapping __sk_common.skc_rx_queue_mapping
>>> +#define sk_rx_ifindex __sk_common.skc_rx_ifindex
>>>
>>> #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin
>>> #define sk_dontcopy_end __sk_common.skc_dontcopy_end
>>> @@ -340,6 +348,7 @@ struct sock {
>>> #define sk_reuseport __sk_common.skc_reuseport
>>> #define sk_ipv6only __sk_common.skc_ipv6only
>>> #define sk_net_refcnt __sk_common.skc_net_refcnt
>>> +#define sk_symmetric_queues __sk_common.skc_symmetric_queues
>>> #define sk_bound_dev_if __sk_common.skc_bound_dev_if
>>> #define sk_bind_node __sk_common.skc_bind_node
>>> #define sk_prot __sk_common.skc_prot
>>> @@ -1676,6 +1685,14 @@ static inline int sk_tx_queue_get(const struct
>>> sock *sk)
>>> return sk ? sk->sk_tx_queue_mapping : -1;
>>> }
>>>
>>> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff
>>> *skb)
>>> +{
>>> + if (sk->sk_symmetric_queues) {
>>> + sk->sk_rx_ifindex = skb->skb_iif;
>>> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
>>> + }
>>> +}
>>> +
>>> static inline void sk_set_socket(struct sock *sk, struct socket *sock)
>>> {
>>> sk_tx_queue_clear(sk);
>>> diff --git a/include/uapi/asm-generic/socket.h
>>> b/include/uapi/asm-generic/socket.h
>>> index e47c9e4..f6b416e 100644
>>> --- a/include/uapi/asm-generic/socket.h
>>> +++ b/include/uapi/asm-generic/socket.h
>>> @@ -106,4 +106,6 @@
>>>
>>> #define SO_ZEROCOPY 60
>>>
>>> +#define SO_SYMMETRIC_QUEUES 61
>>> +
>>> #endif /* __ASM_GENERIC_SOCKET_H */
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 270b547..d96cda8 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -3322,7 +3322,13 @@ static u16 __netdev_pick_tx(struct net_device
>>> *dev, struct sk_buff *skb)
>>>
>>> if (queue_index < 0 || skb->ooo_okay ||
>>> queue_index >= dev->real_num_tx_queues) {
>>> - int new_index = get_xps_queue(dev, skb);
>>> + int new_index = -1;
>>> +
>>> + if (sk && sk->sk_symmetric_queues && dev->ifindex ==
>>> sk->sk_rx_ifindex)
>>> + new_index = sk->sk_rx_queue_mapping;
>>> +
>>> + if (new_index < 0 || new_index >=
>>> dev->real_num_tx_queues)
>>> + new_index = get_xps_queue(dev, skb);
>>
>> This enforces that notion of queue pairs which is not universal
>> concept to NICs. There are many devices and instances where we
>> purposely avoid having a 1-1 relationship between rx and tx queues.
>
>
> Yes. This patch assumes that TX and RX queues come in pairs.
>
>> An alternative might be to create a rx queue to tx queue map, add the
>> rx queue argument to get_xps_queue, and then that function can
>> consider the mapping. The administrator can configure the mapping as
>> appropriate and can select which rx queues are subject to the mapping.
>
> This alternative looks much cleaner and doesn't require the apps to
> configure the
> queues. Do we need to support 1 to many rx to tx queue mappings?
> For our symmetric queues usecase, where a single application thread is
> associated with
> 1 queue-pair, 1-1 mapping is sufficient.
> Do you see any usecase where it is useful to support 1-many mappings?
> I guess i can add a sysfs entry per rx-queue to setup a tx-queue OR
> tx-queue-map.
There is no reason do disallow 1 to many, XPS already does that. In
fact, the mapping algorithm in XSP is pretty much what is needed where
instead of mapping a CPU to a queue set, this just maps a rx queue to
queue set. ooo handling can still be done, although it might be less
critical in this case.
Tom
^ permalink raw reply
* Re: WARNING: CPU: 2 PID: 4277 at lib/refcount.c:186
From: Eric Dumazet @ 2017-09-11 21:58 UTC (permalink / raw)
To: Shankara Pailoor
Cc: LKML, syzkaller, David Miller, Linux Kernel Network Developers,
Cong Wang, Andrew Zhu Aday
In-Reply-To: <CAASgV=txONWMR2_au12ASV40mP0mf6O=4DLoq3+FfGubqG4NQQ@mail.gmail.com>
On Mon, 2017-09-11 at 17:43 -0400, Shankara Pailoor wrote:
> Hi Eric,
>
> I applied your patch and I no longer observed the warning. Thanks! I
> will let you know if any further problems show up during fuzzing.
Great, I also verified this fixed the problem, I will send the official
patch shortly.
Thanks !
^ permalink raw reply
* Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter chain
From: Cong Wang @ 2017-09-11 21:58 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Linux Kernel Network Developers, Jakub Kicinski, Jiri Pirko
In-Reply-To: <CAM_iQpV2p25iZg7sd7jpcN0nuAPmwVUa3-J8F_oGC26aTTPtWw@mail.gmail.com>
On Mon, Sep 11, 2017 at 2:14 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> Looks like all due to the lack of locking on block->chain_list.
> I thought the rcu_barrier() could properly handle this,
> but seems still not, probably I need to move it in the loop,
> I am still not 100% sure if it is totally safe with
> list_for_each_safe():
>
>
> - list_for_each_entry(chain, &block->chain_list, list)
> + list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
> tcf_chain_flush(chain);
> - rcu_barrier();
> + rcu_barrier(); // are we safe now???
> + }
>
Answer myself: No, this is not safe either, because we may
list_del() the next node, and apparently _safe() can't guarantee
that...
So either we have to use locking or use the trick you suggested.
^ permalink raw reply
* Re: WARNING: CPU: 2 PID: 4277 at lib/refcount.c:186
From: Shankara Pailoor @ 2017-09-11 21:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: LKML, syzkaller, David Miller, Linux Kernel Network Developers,
Cong Wang, Andrew Zhu Aday
In-Reply-To: <1505100843.15310.113.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
I applied your patch and I no longer observed the warning. Thanks! I
will let you know if any further problems show up during fuzzing.
Regards,
Shankara
On Sun, Sep 10, 2017 at 11:34 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2017-09-08 at 11:46 -0700, Eric Dumazet wrote:
>> On Fri, 2017-09-08 at 10:21 -0700, Cong Wang wrote:
>> > (Cc'ing netdev)
>> >
>> > On Fri, Sep 8, 2017 at 5:59 AM, Shankara Pailoor <sp3485@columbia.edu> wrote:
>> > > Hi,
>> > >
>> > > I found a warning while fuzzing with Syzkaller on linux 4.13-rc7 on
>> > > x86_64. The full stack trace is below:
>> > >
>> > > WARNING: CPU: 2 PID: 4277 at lib/refcount.c:186
>> > > refcount_sub_and_test+0x167/0x1b0 lib/refcount.c:186
>> > > Kernel panic - not syncing: panic_on_warn set ...
>> > >
>> > > CPU: 2 PID: 4277 Comm: syz-executor0 Not tainted 4.13.0-rc7 #3
>> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>> > > Ubuntu-1.8.2-1ubuntu1 04/01/2014
>> > > Call Trace:
>> > > <IRQ>
>> > > __dump_stack lib/dump_stack.c:16 [inline]
>> > > dump_stack+0xf7/0x1aa lib/dump_stack.c:52
>> > > panic+0x1ae/0x3a7 kernel/panic.c:180
>> > > __warn+0x1c4/0x1d9 kernel/panic.c:541
>> > > report_bug+0x211/0x2d0 lib/bug.c:183
>> > > fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:190
>> > > do_trap_no_signal arch/x86/kernel/traps.c:224 [inline]
>> > > do_trap+0x260/0x390 arch/x86/kernel/traps.c:273
>> > > do_error_trap+0x118/0x340 arch/x86/kernel/traps.c:310
>> > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:323
>> > > invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:846
>> > > RIP: 0010:refcount_sub_and_test+0x167/0x1b0 lib/refcount.c:186
>> > > RSP: 0018:ffff88006e006b60 EFLAGS: 00010286
>> > > RAX: 0000000000000026 RBX: 0000000000000000 RCX: 0000000000000000
>> > > RDX: 0000000000000026 RSI: 1ffff1000dc00d2c RDI: ffffed000dc00d60
>> > > RBP: ffff88006e006bf0 R08: 0000000000000001 R09: 0000000000000000
>> > > R10: 0000000000000000 R11: 0000000000000000 R12: 1ffff1000dc00d6d
>> > > R13: 00000000ffffffff R14: 0000000000000001 R15: ffff88006ce9d340
>> > > refcount_dec_and_test+0x1a/0x20 lib/refcount.c:211
>> > > reqsk_put+0x71/0x2b0 include/net/request_sock.h:123
>> > > tcp_v4_rcv+0x259e/0x2e20 net/ipv4/tcp_ipv4.c:1729
>> > > ip_local_deliver_finish+0x2e2/0xba0 net/ipv4/ip_input.c:216
>> > > NF_HOOK include/linux/netfilter.h:248 [inline]
>> > > ip_local_deliver+0x1ce/0x6d0 net/ipv4/ip_input.c:257
>> > > dst_input include/net/dst.h:477 [inline]
>> > > ip_rcv_finish+0x8db/0x19c0 net/ipv4/ip_input.c:397
>> > > NF_HOOK include/linux/netfilter.h:248 [inline]
>> > > ip_rcv+0xc3f/0x17d0 net/ipv4/ip_input.c:488
>> > > __netif_receive_skb_core+0x1fb7/0x31f0 net/core/dev.c:4298
>> > > __netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4336
>> > > process_backlog+0x1c5/0x6d0 net/core/dev.c:5102
>> > > napi_poll net/core/dev.c:5499 [inline]
>> > > net_rx_action+0x6d3/0x14a0 net/core/dev.c:5565
>> > > __do_softirq+0x2cb/0xb2d kernel/softirq.c:284
>> > > do_softirq_own_stack+0x1c/0x30 arch/x86/entry/entry_64.S:898
>> > > </IRQ>
>> > > do_softirq.part.16+0x63/0x80 kernel/softirq.c:328
>> > > do_softirq kernel/softirq.c:176 [inline]
>> > > __local_bh_enable_ip+0x84/0x90 kernel/softirq.c:181
>> > > local_bh_enable include/linux/bottom_half.h:31 [inline]
>> > > rcu_read_unlock_bh include/linux/rcupdate.h:705 [inline]
>> > > ip_finish_output2+0x8ad/0x1360 net/ipv4/ip_output.c:231
>> > > ip_finish_output+0x74e/0xb80 net/ipv4/ip_output.c:317
>> > > NF_HOOK_COND include/linux/netfilter.h:237 [inline]
>> > > ip_output+0x1cc/0x850 net/ipv4/ip_output.c:405
>> > > dst_output include/net/dst.h:471 [inline]
>> > > ip_local_out+0x95/0x160 net/ipv4/ip_output.c:124
>> > > ip_queue_xmit+0x8c6/0x1810 net/ipv4/ip_output.c:504
>> > > tcp_transmit_skb+0x1963/0x3320 net/ipv4/tcp_output.c:1123
>> > > tcp_send_ack.part.35+0x38c/0x620 net/ipv4/tcp_output.c:3575
>> > > tcp_send_ack+0x49/0x60 net/ipv4/tcp_output.c:3545
>> > > tcp_rcv_synsent_state_process net/ipv4/tcp_input.c:5795 [inline]
>> > > tcp_rcv_state_process+0x4876/0x4b60 net/ipv4/tcp_input.c:5930
>> > > tcp_v4_do_rcv+0x58a/0x820 net/ipv4/tcp_ipv4.c:1483
>> > > sk_backlog_rcv include/net/sock.h:907 [inline]
>> > > __release_sock+0x124/0x360 net/core/sock.c:2223
>> > > release_sock+0xa4/0x2a0 net/core/sock.c:2715
>> > > inet_wait_for_connect net/ipv4/af_inet.c:557 [inline]
>> > > __inet_stream_connect+0x671/0xf00 net/ipv4/af_inet.c:643
>> > > inet_stream_connect+0x58/0xa0 net/ipv4/af_inet.c:682
>> > > SYSC_connect+0x204/0x470 net/socket.c:1628
>> > > SyS_connect+0x24/0x30 net/socket.c:1609
>> > > entry_SYSCALL_64_fastpath+0x18/0xad
>> > > RIP: 0033:0x451e59
>> > > RSP: 002b:00007f474843fc08 EFLAGS: 00000216 ORIG_RAX: 000000000000002a
>> > > RAX: ffffffffffffffda RBX: 0000000000718000 RCX: 0000000000451e59
>> > > RDX: 0000000000000010 RSI: 0000000020002000 RDI: 0000000000000007
>> > > RBP: 0000000000000046 R08: 0000000000000000 R09: 0000000000000000
>> > > R10: 0000000000000000 R11: 0000000000000216 R12: 0000000000000000
>> > > R13: 00007ffc040a0f8f R14: 00007f47484409c0 R15: 0000000000000000
>> > >
>> > >
>> > >
>> > >
>> > > I found that the following program is able to reproduce the warning:
>> > >
>> > >
>> > > Pastebin: https://pastebin.com/B75BdYKz
>> > >
>> > > Here are my configs: https://pastebin.com/zRYCXbak
>> > >
>> > > Regards,
>> > > Shankara
>> > >
>>
>> Sweet, thanks for the report, I will have a look.
>>
>> It seems one reqsk_put(req); is missing, but that would lead to a memory
>> leak, not a double reqsk_put(req) :/
>
> OK, it seems the following patch should fix the issue, can you try it ?
>
> (not compiled yet, I will test this tomorrow)
>
> I will have to double check this wont bring other problems :/
>
> Thanks.
>
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 4089c013cb03b12e31ddffbb7ae903542c012ae0..b9c64b40a83af1e151f553ba0e624fae5060ffd0 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -916,7 +916,6 @@ static void inet_child_forget(struct sock *sk, struct request_sock *req,
> tcp_sk(child)->fastopen_rsk = NULL;
> }
> inet_csk_destroy_sock(child);
> - reqsk_put(req);
> }
>
> struct sock *inet_csk_reqsk_queue_add(struct sock *sk,
> @@ -987,6 +986,7 @@ void inet_csk_listen_stop(struct sock *sk)
> sock_hold(child);
>
> inet_child_forget(sk, req, child);
> + reqsk_put(req);
> bh_unlock_sock(child);
> local_bh_enable();
> sock_put(child);
>
>
^ permalink raw reply
* Re: [PATCH] net/sched: fix pointer check in gen_handle
From: David Miller @ 2017-09-11 21:35 UTC (permalink / raw)
To: johunt; +Cc: jhs, xiyou.wangcong, jiri, netdev
In-Reply-To: <1505072930-13049-1-git-send-email-johunt@akamai.com>
From: Josh Hunt <johunt@akamai.com>
Date: Sun, 10 Sep 2017 15:48:50 -0400
> Fixes sparse warning about pointer in gen_handle:
> net/sched/cls_rsvp.h:392:40: warning: Using plain integer as NULL pointer
>
> Fixes: 8113c095672f6 ("net_sched: use void pointer for filter handle")
> Signed-off-by: Josh Hunt <johunt@akamai.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] ipv6: sr: remove duplicate routing header type check
From: David Miller @ 2017-09-11 21:34 UTC (permalink / raw)
To: dav.lebrun; +Cc: netdev
In-Reply-To: <20170910132201.15576-1-dav.lebrun@gmail.com>
From: David Lebrun <dav.lebrun@gmail.com>
Date: Sun, 10 Sep 2017 14:22:01 +0100
> From: David Lebrun <dlebrun@google.com>
>
> As seg6_validate_srh() already checks that the Routing Header type is
> correct, it is not necessary to do it again in get_srh().
>
> Fixes: 5829d70b ("ipv6: sr: fix get_srh() to comply with IPv6 standard "RFC 8200")
> Signed-off-by: David Lebrun <dlebrun@google.com>
Applied thanks.
^ permalink raw reply
* Re: [V3 PATCH net] xdp: implement xdp_redirect_map for generic XDP
From: David Miller @ 2017-09-11 21:33 UTC (permalink / raw)
To: brouer; +Cc: netdev, john.fastabend, andy
In-Reply-To: <150502962259.28817.7472180087690436639.stgit@firesoul>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Sun, 10 Sep 2017 09:47:02 +0200
> Using bpf_redirect_map is allowed for generic XDP programs, but the
> appropriate map lookup was never performed in xdp_do_generic_redirect().
>
> Instead the map-index is directly used as the ifindex. For the
> xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
> sending on ifindex 0 which isn't valid, resulting in getting SKB
> packets dropped. Thus, the reported performance numbers are wrong in
> commit 24251c264798 ("samples/bpf: add option for native and skb mode
> for redirect apps") for the 'xdp_redirect_map -S' case.
>
> Before commit 109980b894e9 ("bpf: don't select potentially stale
> ri->map from buggy xdp progs") it could crash the kernel. Like this
> commit also check that the map_owner owner is correct before
> dereferencing the map pointer. But make sure that this API misusage
> can be caught by a tracepoint. Thus, allowing userspace via
> tracepoints to detect misbehaving bpf_progs.
>
> Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
> Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect apps")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Applied, thanks Jesper.
^ permalink raw reply
* Re: Extended IDR API
From: David Miller @ 2017-09-11 21:31 UTC (permalink / raw)
To: willy; +Cc: chrism, jiri, tj, linux-kernel, netdev, aquannie
In-Reply-To: <20170911211408.GA26747@bombadil.infradead.org>
From: Matthew Wilcox <willy@infradead.org>
Date: Mon, 11 Sep 2017 14:14:08 -0700
> I really don't like your new API. I wish you'd discussed it before
> merging it.
If I recall correctly, linux-kernel was CC:'d and nobody engaged.
I could be wrong, and if I am then my bad, I should have enforced that.
Thanks.
^ permalink raw reply
* Re: [PATCH] tipc: Use bsearch library function
From: David Miller @ 2017-09-11 21:30 UTC (permalink / raw)
To: thomas; +Cc: jon.maloy, ying.xue, netdev, tipc-discussion, linux-kernel
In-Reply-To: <20170909031819.13622-1-thomas@m3y3r.de>
From: Thomas Meyer <thomas@m3y3r.de>
Date: Sat, 9 Sep 2017 05:18:19 +0200
> @@ -168,6 +169,18 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
> return nseq;
> }
>
> +static int nameseq_find_subseq_cmp(const void *key, const void *elt)
> +{
> + u32 instance = *(u32 *)key;
> + struct sub_seq *sseq = (struct sub_seq *)elt;
Please order local variables from longest to shortest (ie. reverse
christmas tree).
Thank you.
^ permalink raw reply
* Re: [PATCH net] perf/bpf: fix a clang compilation issue
From: David Miller @ 2017-09-11 21:28 UTC (permalink / raw)
To: yhs; +Cc: peterz, rostedt, ast, daniel, ndesaulniers, netdev, kernel-team
In-Reply-To: <20170908013615.973478-1-yhs@fb.com>
From: Yonghong Song <yhs@fb.com>
Date: Thu, 7 Sep 2017 18:36:15 -0700
> clang does not support variable length array for structure member.
> It has the following error during compilation:
>
> kernel/trace/trace_syscalls.c:568:17: error: fields must have a constant size:
> 'variable length array in structure' extension will never be supported
> unsigned long args[sys_data->nb_args];
> ^
>
> The fix is to use a fixed array length instead.
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: David Miller @ 2017-09-11 21:26 UTC (permalink / raw)
To: tatsu; +Cc: j.vosburgh, vfalico, andy, netdev, linux-kernel, r
In-Reply-To: <17EC94B0A072C34B8DCF0D30AD16044A02985CB5@BPXM09GP.gisp.nec.co.jp>
From: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
Date: Wed, 6 Sep 2017 22:47:59 +0000
> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
> balance-alb mode") tried to fix transmit dynamic load balancing in
> balance-alb mode, which wasn't working after commit 8b426dc54cf4
> ("bonding: remove hardcoded value").
>
> It turned out that my previous patch only fixed the case when
> balance-alb was specified as bonding module parameter, and not when
> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
> common usage). In the latter case, tlb_dynamic_lb was set up according
> to the default mode of the bonding interface, which happens to be
> balance-rr.
>
> This additional patch addresses this issue by setting up tlb_dynamic_lb
> to 1 if "mode" is set to balance-alb through the sysfs interface.
>
> I didn't add code to change tlb_balance_lb back to the default value for
> other modes, because "mode" is usually set up only once during
> initialization, and it's not worthwhile to change the static variable
> bonding_defaults in bond_main.c to a global variable just for this
> purpose.
>
> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
> balance-tlb mode if it is set up using the sysfs interface. I didn't
> change that behavior, because the value of tlb_balance_lb can be changed
> using the sysfs interface for balance-tlb, and I didn't like changing
> the default value back and forth for balance-tlb.
>
> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
> is not an intended usage, so there is little use making it writable at
> this moment.
>
> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
> Reported-by: Reinis Rozitis <r@roze.lv>
> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
> Cc: stable@vger.kernel.org # v4.12+
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] hv_netvsc: avoid unnecessary wakeups on subchannel creation
From: David Miller @ 2017-09-11 21:21 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20170906205306.10541-3-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 6 Sep 2017 13:53:06 -0700
> Only need to wakeup the initiator after all sub-channels
> are opened.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/2] hv_netvsc: fix deadlock on hotplug
From: David Miller @ 2017-09-11 21:21 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20170906205306.10541-2-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 6 Sep 2017 13:53:05 -0700
> When a virtual device is added dynamically (via host console), then
> the vmbus sends an offer message for the primary channel. The processing
> of this message for networking causes the network device to then
> initialize the sub channels.
>
> The problem is that setting up the sub channels needs to wait until
> the subsequent subchannel offers have been processed. These offers
> come in on the same ring buffer and work queue as where the primary
> offer is being processed; leading to a deadlock.
>
> This did not happen in older kernels, because the sub channel waiting
> logic was broken (it wasn't really waiting).
>
> The solution is to do the sub channel setup in its own work queue
> context that is scheduled by the primary channel setup; and then
> happens later.
>
> Fixes: 732e49850c5e ("netvsc: fix race on sub channel creation")
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> v2 - fix module removal race with new work queue
Applied.
^ permalink raw reply
* Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter chain
From: Cong Wang @ 2017-09-11 21:14 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Linux Kernel Network Developers, Jakub Kicinski, Jiri Pirko
In-Reply-To: <20170910143300.GB1860@nanopsycho>
On Sun, Sep 10, 2017 at 7:33 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Sep 08, 2017 at 06:37:55PM CEST, xiyou.wangcong@gmail.com wrote:
>>On Thu, Sep 7, 2017 at 1:52 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangcong@gmail.com wrote:
>>>>Yes it is for chain 0, because block holds a reference to chain 0 during
>>>>creation. Non-0 chains are created with refcnt==1 too but paired with
>>>>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0
>>>>not special w.r.t. refcnt.
>>>
>>> So you need to tcf_chain_put only chain 0 here, right? The rest of the
>>> chains get destroyed by the previous list_for_each_entry iteration when
>>> flush happens and actions destroy happens what decrements recnt to 0
>>> there.
>>
>>
>>This is correct. And it should be only chain 0 after flush.
>>
>>>
>>> What do I miss, who would still hold reference for non-0 chains when all
>>> tps and all goto_chain actions are gone?
>>
>>No one. This is totally correct and is exactly what this patch intends to do.
>>
>>Look, this is why we never need an object with refcnt==0 to exist. ;)
>
> So, I understand that correctly, good. But this is a problem.
>
> When you do:
> list_for_each_entry(chain, &block->chain_list, list)
> tcf_chain_flush(chain);
>
> The reference may get dropped for chains to 0 (for those that does not
> have a goto_chain action holding a ref), and therefore they get freed
> within the loop. That is problematic when you do the traversing of the
> list. You may use list_for_each_entry_safe, but there is another issue:
>
> As a part of tcf_chain_flush destruction, act goto_chain destruction may
> get scheduled by call_rcu. That may be the last reference held for the
> chain. So you race between this loop and rcu callback.
>
> Consider following example:
>
> chain0 - has only one rule with goto_chain 22 action
> chain22 - no rule (refcnt 1 because of the action mentioned above)
>
> CPU0 CPU1
>
> tcf_chain_flush(0)
> -> call_rcu(free_tcf)
> free_tcf
> ->tcf_chain_put(22)
> ->tcf_chain_destroy(22)
> ->kfree(22)
Looks like all due to the lack of locking on block->chain_list.
I thought the rcu_barrier() could properly handle this,
but seems still not, probably I need to move it in the loop,
I am still not 100% sure if it is totally safe with
list_for_each_safe():
- list_for_each_entry(chain, &block->chain_list, list)
+ list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
tcf_chain_flush(chain);
- rcu_barrier();
+ rcu_barrier(); // are we safe now???
+ }
> tcf_chain_flush(22)...use-after-free
>
Same race could happen with your code too, right?
chain 22 still has refcnt==1 so chain_put() will destroy
it in flush() too. So this is not a regression.
I know you have list_for_each_safe() but you lack of
rcu_barrier(). This is why I said the lack of locking is
the cause, not your code or my code.
>
> So what I suggest in order to prevent this is to change your code to
> something like:
>
> /* To avoid race between getting reference in the next loop and
> * rcu callbacks from deleleted actions freeing the chain.
> */
> rcu_barrier();
>
> list_for_each_entry(chain, &block->chain_list, list)
> if (chain->index) /* we already hold ref to chain 0 */
> tcf_chain_hold(chain);
>
> list_for_each_entry(chain, &block->chain_list, list)
> tcf_chain_flush(chain);
>
> /* Wait for rcu callbacks from deleleted actions that were
> * sheduled as a result of tcf_chain_flush in the previous loop.
> * This is not absolutelly necessary, as the chain may live after
> * the tcf_chain_put is called in the next iteration and would
> * get freed on tcf_chain_put call from rcu callback later on.
> */
> rcu_barrier();
>
> /* Now we are sure that we are the only one holding a reference
> * to all chains, drop it and let them go.
> */
> list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
> tcf_chain_put(chain);
> kfree(block);
>
> Does this make sense?
Yeah, but again this is all due to lack of locking. Do we really
have to to be such complex or just use either a) proper sync
with rcu_barrier() or b) proper locking on chain_list (with RCU
of course)?
Thanks.
^ permalink raw reply
* Extended IDR API
From: Matthew Wilcox @ 2017-09-11 21:14 UTC (permalink / raw)
To: Chris Mi
Cc: Jiri Pirko, David S. Miller, Tejun Heo, linux-kernel, netdev,
Rehas Sachdeva
I really don't like your new API. I wish you'd discussed it before
merging it. Here's my redesign. Does anybody have a suggestion for
improvement?
We have a lovely new test-suite for the IDR (in tools/testing/radix-tree)
... when adding a new API, it's polite to update the test-suite too.
Do you have any plans to add test cases?
(Compile tested only; I'm at a conference. Also, I didn't check the
kerneldoc because I don't have Sphinx installed on my laptop.)
>From ff45b2a6806cd0e4177c5a10f26c97999164c10c Mon Sep 17 00:00:00 2001
From: Matthew Wilcox <mawilcox@microsoft.com>
Date: Mon, 11 Sep 2017 16:16:29 -0400
Subject: [PATCH] idr: Rewrite extended IDR API
- Rename the API to be 'ul' for unsigned long instead of 'ext'. This
fits better with other usage in the Linux kernel.
- idr_alloc() moves back to being a function instead of inline
- idr_alloc_ul() takes 'nextid' as an in-out parameter like idr_get_next(),
instead of having 'index' as an out-only parameter.
- idr_alloc_ul() needs a __must_check to ensure that users don't look at
the result without checking whether the function succeeded.
- idr_alloc_ul() takes 'max' rather than 'end', or it is impossible to
allocate the ULONG_MAX id.
- idr_replace() can simply take an unsigned long parameter instead of
an int.
- idr_remove() and idr_find() are the same as idr_replace().
- We don't need separate idr_get_free() and idr_get_free_ext().
- Add kerneldoc for idr_alloc_ul().
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 75 +++++----------------------------------
include/linux/radix-tree.h | 17 +--------
lib/idr.c | 88 +++++++++++++++++++++++++++++++++-------------
lib/radix-tree.c | 2 +-
net/sched/act_api.c | 22 ++++++------
net/sched/cls_flower.c | 16 +++++----
6 files changed, 95 insertions(+), 125 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 7c3a365f7e12..90faf8279559 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -81,74 +81,22 @@ static inline void idr_set_cursor(struct idr *idr, unsigned int val)
void idr_preload(gfp_t gfp_mask);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext);
-
-/**
- * idr_alloc - allocate an id
- * @idr: idr handle
- * @ptr: pointer to be associated with the new id
- * @start: the minimum id (inclusive)
- * @end: the maximum id (exclusive)
- * @gfp: memory allocation flags
- *
- * Allocates an unused ID in the range [start, end). Returns -ENOSPC
- * if there are no unused IDs in that range.
- *
- * Note that @end is treated as max when <= 0. This is to always allow
- * using @start + N as @end as long as N is inside integer range.
- *
- * Simultaneous modifications to the @idr are not allowed and should be
- * prevented by the user, usually with a lock. idr_alloc() may be called
- * concurrently with read-only accesses to the @idr, such as idr_find() and
- * idr_for_each_entry().
- */
-static inline int idr_alloc(struct idr *idr, void *ptr,
- int start, int end, gfp_t gfp)
-{
- unsigned long id;
- int ret;
-
- if (WARN_ON_ONCE(start < 0))
- return -EINVAL;
-
- ret = idr_alloc_cmn(idr, ptr, &id, start, end, gfp, false);
-
- if (ret)
- return ret;
-
- return id;
-}
-
-static inline int idr_alloc_ext(struct idr *idr, void *ptr,
- unsigned long *index,
- unsigned long start,
- unsigned long end,
- gfp_t gfp)
-{
- return idr_alloc_cmn(idr, ptr, index, start, end, gfp, true);
-}
-
+int idr_alloc(struct idr *, void *, int start, int end, gfp_t);
+int __must_check idr_alloc_ul(struct idr *, void *, unsigned long *nextid,
+ unsigned long max, gfp_t);
int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t);
int idr_for_each(const struct idr *,
int (*fn)(int id, void *p, void *data), void *data);
void *idr_get_next(struct idr *, int *nextid);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
-void *idr_replace(struct idr *, void *, int id);
-void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
+void *idr_get_next_ul(struct idr *, unsigned long *nextid);
+void *idr_replace(struct idr *, void *, unsigned long id);
void idr_destroy(struct idr *);
-static inline void *idr_remove_ext(struct idr *idr, unsigned long id)
+static inline void *idr_remove(struct idr *idr, unsigned long id)
{
return radix_tree_delete_item(&idr->idr_rt, id, NULL);
}
-static inline void *idr_remove(struct idr *idr, int id)
-{
- return idr_remove_ext(idr, id);
-}
-
static inline void idr_init(struct idr *idr)
{
INIT_RADIX_TREE(&idr->idr_rt, IDR_RT_MARKER);
@@ -184,16 +132,11 @@ static inline void idr_preload_end(void)
* This function can be called under rcu_read_lock(), given that the leaf
* pointers lifetimes are correctly managed.
*/
-static inline void *idr_find_ext(const struct idr *idr, unsigned long id)
+static inline void *idr_find(const struct idr *idr, unsigned long id)
{
return radix_tree_lookup(&idr->idr_rt, id);
}
-static inline void *idr_find(const struct idr *idr, int id)
-{
- return idr_find_ext(idr, id);
-}
-
/**
* idr_for_each_entry - iterate over an idr's elements of a given type
* @idr: idr handle
@@ -206,8 +149,8 @@ static inline void *idr_find(const struct idr *idr, int id)
*/
#define idr_for_each_entry(idr, entry, id) \
for (id = 0; ((entry) = idr_get_next(idr, &(id))) != NULL; ++id)
-#define idr_for_each_entry_ext(idr, entry, id) \
- for (id = 0; ((entry) = idr_get_next_ext(idr, &(id))) != NULL; ++id)
+#define idr_for_each_entry_ul(idr, entry, id) \
+ for (id = 0; ((entry) = idr_get_next_ul(idr, &(id))) != NULL; ++id)
/**
* idr_for_each_entry_continue - continue iteration over an idr's elements of a given type
diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 567ebb5eaab0..8275fc2ed0f4 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -358,24 +358,9 @@ int radix_tree_split(struct radix_tree_root *, unsigned long index,
int radix_tree_join(struct radix_tree_root *, unsigned long index,
unsigned new_order, void *);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max);
-static inline void __rcu **idr_get_free(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- int end)
-{
- return idr_get_free_cmn(root, iter, gfp, end > 0 ? end - 1 : INT_MAX);
-}
-
-static inline void __rcu **idr_get_free_ext(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- unsigned long end)
-{
- return idr_get_free_cmn(root, iter, gfp, end - 1);
-}
enum {
RADIX_TREE_ITER_TAG_MASK = 0x0f, /* tag index in lower nybble */
diff --git a/lib/idr.c b/lib/idr.c
index 082778cf883e..230879a65d99 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -7,9 +7,26 @@
DEFINE_PER_CPU(struct ida_bitmap *, ida_bitmap);
static DEFINE_SPINLOCK(simple_ida_lock);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext)
+/**
+ * idr_alloc_ul - allocate a large ID
+ * @idr: idr handle
+ * @ptr: pointer to be associated with the new ID
+ * @nextid: Pointer to minimum ID to allocate
+ * @max: the maximum ID (inclusive)
+ * @gfp: memory allocation flags
+ *
+ * Allocates an unused ID in the range [*nextid, end] and stores it in
+ * @nextid. Note that @max differs from the @end parameter to idr_alloc().
+ *
+ * Simultaneous modifications to the @idr are not allowed and should be
+ * prevented by the user, usually with a lock. idr_alloc_ul() may be called
+ * concurrently with read-only accesses to the @idr, such as idr_find() and
+ * idr_for_each_entry().
+ *
+ * Return: 0 on success or a negative errno on failure (ENOMEM or ENOSPC)
+ */
+int idr_alloc_ul(struct idr *idr, void *ptr, unsigned long *nextid,
+ unsigned long max, gfp_t gfp)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -17,22 +34,54 @@ int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
return -EINVAL;
- radix_tree_iter_init(&iter, start);
- if (ext)
- slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
- else
- slot = idr_get_free(&idr->idr_rt, &iter, gfp, end);
+ radix_tree_iter_init(&iter, *nextid);
+ slot = idr_get_free(&idr->idr_rt, &iter, gfp, max);
if (IS_ERR(slot))
return PTR_ERR(slot);
radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
- if (index)
- *index = iter.index;
+ *nextid = iter.index;
return 0;
}
-EXPORT_SYMBOL_GPL(idr_alloc_cmn);
+EXPORT_SYMBOL_GPL(idr_alloc_ul);
+
+/**
+ * idr_alloc - allocate an id
+ * @idr: idr handle
+ * @ptr: pointer to be associated with the new id
+ * @start: the minimum id (inclusive)
+ * @end: the maximum id (exclusive)
+ * @gfp: memory allocation flags
+ *
+ * Allocates an unused ID in the range [start, end). Returns -ENOSPC
+ * if there are no unused IDs in that range.
+ *
+ * Note that @end is treated as max when <= 0. This is to always allow
+ * using @start + N as @end as long as N is inside integer range.
+ *
+ * Simultaneous modifications to the @idr are not allowed and should be
+ * prevented by the user, usually with a lock. idr_alloc() may be called
+ * concurrently with read-only accesses to the @idr, such as idr_find() and
+ * idr_for_each_entry().
+ */
+int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
+{
+ unsigned long id = start;
+ int ret;
+
+ if (WARN_ON_ONCE(start < 0))
+ return -EINVAL;
+
+ ret = idr_alloc_ul(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
+
+ if (ret)
+ return ret;
+
+ return id;
+}
+EXPORT_SYMBOL_GPL(idr_alloc);
/**
* idr_alloc_cyclic - allocate new idr entry in a cyclical fashion
@@ -121,7 +170,7 @@ void *idr_get_next(struct idr *idr, int *nextid)
}
EXPORT_SYMBOL(idr_get_next);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
+void *idr_get_next_ul(struct idr *idr, unsigned long *nextid)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -133,7 +182,7 @@ void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
*nextid = iter.index;
return rcu_dereference_raw(*slot);
}
-EXPORT_SYMBOL(idr_get_next_ext);
+EXPORT_SYMBOL(idr_get_next_ul);
/**
* idr_replace - replace pointer for given id
@@ -149,16 +198,7 @@ EXPORT_SYMBOL(idr_get_next_ext);
* Returns: 0 on success. %-ENOENT indicates that @id was not found.
* %-EINVAL indicates that @id or @ptr were not valid.
*/
-void *idr_replace(struct idr *idr, void *ptr, int id)
-{
- if (WARN_ON_ONCE(id < 0))
- return ERR_PTR(-EINVAL);
-
- return idr_replace_ext(idr, ptr, id);
-}
-EXPORT_SYMBOL(idr_replace);
-
-void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
+void *idr_replace(struct idr *idr, void *ptr, unsigned long id)
{
struct radix_tree_node *node;
void __rcu **slot = NULL;
@@ -175,7 +215,7 @@ void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
return entry;
}
-EXPORT_SYMBOL(idr_replace_ext);
+EXPORT_SYMBOL(idr_replace);
/**
* DOC: IDA description
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 8b1feca1230a..9fcd4e5c5237 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -2139,7 +2139,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
}
EXPORT_SYMBOL(ida_pre_get);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max)
{
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a306974e2fb4..131817ab3ad3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -73,7 +73,7 @@ static void free_tcf(struct rcu_head *head)
static void tcf_idr_remove(struct tcf_idrinfo *idrinfo, struct tc_action *p)
{
spin_lock_bh(&idrinfo->lock);
- idr_remove_ext(&idrinfo->action_idr, p->tcfa_index);
+ idr_remove(&idrinfo->action_idr, p->tcfa_index);
spin_unlock_bh(&idrinfo->lock);
gen_kill_estimator(&p->tcfa_rate_est);
/*
@@ -121,7 +121,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
s_i = cb->args[0];
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
index++;
if (index < s_i)
continue;
@@ -178,7 +178,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
if (nla_put_string(skb, TCA_KIND, ops->kind))
goto nla_put_failure;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED) {
module_put(p->ops->owner);
@@ -216,10 +216,10 @@ EXPORT_SYMBOL(tcf_generic_walker);
static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
{
- struct tc_action *p = NULL;
+ struct tc_action *p;
spin_lock_bh(&idrinfo->lock);
- p = idr_find_ext(&idrinfo->action_idr, index);
+ p = idr_find(&idrinfo->action_idr, index);
spin_unlock_bh(&idrinfo->lock);
return p;
@@ -296,10 +296,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
spin_lock_init(&p->tcfa_lock);
/* user doesn't specify an index */
if (!index) {
+ idr_index = 1;
idr_preload(GFP_KERNEL);
spin_lock_bh(&idrinfo->lock);
- err = idr_alloc_ext(idr, NULL, &idr_index, 1, 0,
- GFP_ATOMIC);
+ err = idr_alloc_ul(idr, NULL, &idr_index, UINT_MAX, GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
idr_preload_end();
if (err) {
@@ -309,10 +309,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
}
p->tcfa_index = idr_index;
} else {
+ idr_index = index;
idr_preload(GFP_KERNEL);
spin_lock_bh(&idrinfo->lock);
- err = idr_alloc_ext(idr, NULL, NULL, index, index + 1,
- GFP_ATOMIC);
+ err = idr_alloc_ul(idr, NULL, &idr_index, index, GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
idr_preload_end();
if (err)
@@ -345,7 +345,7 @@ void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a)
struct tcf_idrinfo *idrinfo = tn->idrinfo;
spin_lock_bh(&idrinfo->lock);
- idr_replace_ext(&idrinfo->action_idr, a, a->tcfa_index);
+ idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
spin_unlock_bh(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_insert);
@@ -358,7 +358,7 @@ void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
int ret;
unsigned long id = 1;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED)
module_put(ops->owner);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 1a267e77c6de..ad30968f587d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -298,7 +298,7 @@ static void __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f)
{
struct cls_fl_head *head = rtnl_dereference(tp->root);
- idr_remove_ext(&head->handle_idr, f->handle);
+ idr_remove(&head->handle_idr, f->handle);
list_del_rcu(&f->list);
if (!tc_skip_hw(f->flags))
fl_hw_destroy_filter(tp, f);
@@ -341,7 +341,7 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
{
struct cls_fl_head *head = rtnl_dereference(tp->root);
- return idr_find_ext(&head->handle_idr, handle);
+ return idr_find(&head->handle_idr, handle);
}
static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
@@ -901,8 +901,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
goto errout;
if (!handle) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- 1, 0x80000000, GFP_KERNEL);
+ idr_index = 1;
+ err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
+ INT_MAX, GFP_KERNEL);
if (err)
goto errout;
fnew->handle = idr_index;
@@ -910,8 +911,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
/* user specifies a handle and it doesn't exist */
if (handle && !fold) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- handle, handle + 1, GFP_KERNEL);
+ idr_index = handle;
+ err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
+ handle, GFP_KERNEL);
if (err)
goto errout;
fnew->handle = idr_index;
@@ -970,7 +972,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
if (fold) {
fnew->handle = handle;
- idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
+ idr_replace(&head->handle_idr, fnew, fnew->handle);
list_replace_rcu(&fold->list, &fnew->list);
tcf_unbind_filter(tp, &fold->res);
call_rcu(&fold->rcu, fl_destroy_filter);
--
2.14.1
^ permalink raw reply related
* Re: [net-next PATCHv6 1/2] dt-bindings: net: Add DT bindings for Socionext Netsec
From: Rob Herring @ 2017-09-11 20:37 UTC (permalink / raw)
To: Jassi Brar
Cc: netdev, devicetree, linux-arm-kernel, davem, patches, arnd,
mark.rutland, andy, Jassi Brar
In-Reply-To: <1504088752-6204-1-git-send-email-jaswinder.singh@linaro.org>
On Wed, Aug 30, 2017 at 03:55:52PM +0530, Jassi Brar wrote:
> This patch adds documentation for Device-Tree bindings for the
> Socionext NetSec Controller driver.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
> .../devicetree/bindings/net/socionext-netsec.txt | 46 ++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/socionext-netsec.txt
>
> diff --git a/Documentation/devicetree/bindings/net/socionext-netsec.txt b/Documentation/devicetree/bindings/net/socionext-netsec.txt
> new file mode 100644
> index 0000000..12d596c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext-netsec.txt
> @@ -0,0 +1,46 @@
> +* Socionext NetSec Ethernet Controller IP
> +
> +Required properties:
> +- compatible: Should be "socionext,netsecv5"
This should be SoC specific, or also have an SoC specific string.
> +- reg: Address and length of the register sets, the first is the main
> + registers, then the rdlar and tdlar regions for the SoC
> +- interrupts: Should contain ethernet controller interrupt
> +- clocks: phandle to any clocks to be switched by runtime_pm
> +- phy-mode: See ethernet.txt file in the same directory
> +- max-speed: See ethernet.txt file in the same directory
> +- max-frame-size: See ethernet.txt file in the same directory, if 9000 or
> + above jumbo frames are enabled
> +- local-mac-address: See ethernet.txt file in the same directory
> +- phy-handle: phandle to select child phy
> +
> +Optional properties:
> +- use-jumbo: Boolean property to suggest if jumbo packets should be used or not
Is this common or vendor specific. The former should refer to a common
doc. The latter should have a vendor prefix. I'd think the former makes
sense here.
But then why do you need this and max-frame-size?
> +
> +For the child phy
> +
> +- compatible "ethernet-phy-ieee802.3-c22" is needed
> +- device_type "ethernet-phy"
> +- reg: phy address
> +
> +
> +Example:
> + eth0: ethernet {
> + compatible = "socionext,netsecv5";
> + reg = <0 0x31600000 0x10000>, <0 0x31618000 0x4000>, <0 0x3161c000 0x4000>;
> + interrupts = <0 163 0x4>;
> + clocks = <&clk_alw_0_8>;
> + phy-mode = "rgmii";
> + max-speed = <1000>;
> + max-frame-size = <9000>;
> + local-mac-address = [ a4 17 31 00 00 ed ];
> + phy-handle = <ðphy0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethphy0: ethernet-phy@1 {
> + device_type = "ethernet-phy";
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + };
> + };
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" 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
* Re: [PATCH v2 net 1/3] lan78xx: Fix for eeprom read/write when device auto suspend
From: Andrew Lunn @ 2017-09-11 20:24 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA335C84@CHN-SV-EXMX03.mchp-main.com>
On Mon, Sep 11, 2017 at 06:51:49PM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Fix for eeprom read/write when device auto suspend
>
> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
> ---
> drivers/net/usb/lan78xx.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index b99a7fb..baf91c7 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1265,30 +1265,44 @@ static int lan78xx_ethtool_get_eeprom(struct net_device *netdev,
> struct ethtool_eeprom *ee, u8 *data)
> {
> struct lan78xx_net *dev = netdev_priv(netdev);
> + int ret = -EINVAL;
> +
> + if (usb_autopm_get_interface(dev->intf) < 0)
> + return ret;
Hi Nisar
It is better to do
ret = usb_autopm_get_interface(dev->intf;
if (ret)
return ret;
i.e. use the error code usb_autopm_get_interface() gives you.
> ee->magic = LAN78XX_EEPROM_MAGIC;
>
> - return lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
> + ret = lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
> +
> + usb_autopm_put_interface(dev->intf);
> +
> + return ret;
> }
>
> static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
> struct ethtool_eeprom *ee, u8 *data)
> {
> struct lan78xx_net *dev = netdev_priv(netdev);
> + int ret = -EINVAL;
> +
> + if (usb_autopm_get_interface(dev->intf) < 0)
> + return ret;
Same here.
Andrew
^ permalink raw reply
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Andrew Lunn @ 2017-09-11 20:19 UTC (permalink / raw)
To: Corentin Labbe
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170911190850.GA2291@Red>
> Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
> So no the CLK/RST are really for the PHY.
Thanks for trying that.
You said it was probably during scanning of the bus it times out. What
address is causing the timeout? 0 or 1? If the internal bus can only
have one PHY on it, maybe we need to set bus->phy_mask to 0x1?
Andrew
--
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
* RE: [PATCH v2 net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Woojung.Huh @ 2017-09-11 20:13 UTC (permalink / raw)
To: Nisar.Sayed, davem; +Cc: UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA335C94@CHN-SV-EXMX03.mchp-main.com>
Nisar,
>@@ -1290,8 +1290,8 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
>
> /* Allow entire eeprom update only */
> if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
>- (ee->offset == 0) &&
>- (ee->len == 512) &&
>+ (ee->offset >= 0 && ee->offset < MAX_EEPROM_SIZE) &&
>+ (ee->len > 0 && (ee->offset + ee->len) <= MAX_EEPROM_SIZE) &&
> (data[0] == EEPROM_INDICATOR))
This patch is for writing any len at any offset.
However, "(data[0] == EEPROM_INDICATOR)" prevents it.
> ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
> else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
- Woojung
^ permalink raw reply
* Re: [PATCH] netfilter: ipset: ipset list may return wrong member count for set with timeout
From: Vishwanath Pai @ 2017-09-11 19:57 UTC (permalink / raw)
To: Jozsef Kadlecsik
Cc: Pablo Neira Ayuso, johunt, pai.vishwain, netfilter-devel,
coreteam, netdev
In-Reply-To: <alpine.DEB.2.11.1709112134060.12411@blackhole.kfki.hu>
Hi Jozsef,
Thank you. Yes, that is true, the count can still be incorrect in the
case of a huge set.
Thanks,
Vishwanath
On 09/11/2017 03:36 PM, Jozsef Kadlecsik wrote:
> Hi,
>
> Your patch is applied in the ipset git tree and I'm going to push it for
> kernel inclusion.
>
> I modified the comment part: the elements counter can still be incorrect
> in the case of a huge set, because elements might time out during the
> listing.
>
> Thanks for your patience!
>
> Best regards,
> Jozsef
>
> On Thu, 17 Aug 2017, Vishwanath Pai wrote:
>
>> Simple testcase:
>>
>> $ ipset create test hash:ip timeout 5
>> $ ipset add test 1.2.3.4
>> $ ipset add test 1.2.2.2
>> $ sleep 5
>>
>> $ ipset l
>> Name: test
>> Type: hash:ip
>> Revision: 5
>> Header: family inet hashsize 1024 maxelem 65536 timeout 5
>> Size in memory: 296
>> References: 0
>> Number of entries: 2
>> Members:
>>
>> We return "Number of entries: 2" but no members are listed. That is
>> because mtype_list runs "ip_set_timeout_expired" and does not list the
>> expired entries, but set->elements is never upated (until mtype_gc
>> cleans it up later).
>>
>> Reviewed-by: Joshua Hunt <johunt@akamai.com>
>> Signed-off-by: Vishwanath Pai <vpai@akamai.com>
>> ---
>> net/netfilter/ipset/ip_set_hash_gen.h | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
>> index f236c0b..ff3d31c 100644
>> --- a/net/netfilter/ipset/ip_set_hash_gen.h
>> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
>> @@ -1041,12 +1041,22 @@ struct htype {
>> static int
>> mtype_head(struct ip_set *set, struct sk_buff *skb)
>> {
>> - const struct htype *h = set->data;
>> + struct htype *h = set->data;
>> const struct htable *t;
>> struct nlattr *nested;
>> size_t memsize;
>> u8 htable_bits;
>>
>> + /* If any members have expired, set->elements will be wrong
>> + * mytype_expire function will update it with the right count.
>> + * we do not hold set->lock here, so grab it first.
>> + */
>> + if (SET_WITH_TIMEOUT(set)) {
>> + spin_lock_bh(&set->lock);
>> + mtype_expire(set, h);
>> + spin_unlock_bh(&set->lock);
>> + }
>> +
>> rcu_read_lock_bh();
>> t = rcu_dereference_bh_nfnl(h->table);
>> memsize = mtype_ahash_memsize(h, t) + set->ext_size;
>> --
>> 1.9.1
>>
>>
>
> -
> E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
> PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
> Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
> H-1525 Budapest 114, POB. 49, Hungary
>
^ permalink raw reply
* [PATCH v2] openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Christophe JAILLET @ 2017-09-11 19:56 UTC (permalink / raw)
To: pshelar, davem, xiangxia.m.yue
Cc: netdev, dev, linux-kernel, kernel-janitors, Christophe JAILLET
All other error handling paths in this function go through the 'error'
label. This one should do the same.
Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
I think that the comment above the function could be improved. It looks
like the commit log which has introduced this function.
I'm also not sure that commit 9cc9a5cb176c is of any help. It is
supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
used by each function is reduced, the overall stack should be the same, if
not larger.
So this commit sounds like adding a bug where the code was fine and states
to fix an issue but, at the best, only hides it.
Instead of fixing the code with the proposed patch, reverting the initial
commit could also be considered.
V2: update Subject line
---
net/openvswitch/datapath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 76cf273a56c7..c3aec6227c91 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
"Flow key attribute not present in set flow.");
- return -EINVAL;
+ error = -EINVAL;
+ goto error;
}
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] netfilter: ipset: ipset list may return wrong member count for set with timeout
From: Jozsef Kadlecsik @ 2017-09-11 19:36 UTC (permalink / raw)
To: Vishwanath Pai
Cc: Pablo Neira Ayuso, johunt, pai.vishwain, netfilter-devel,
coreteam, netdev
In-Reply-To: <1502947435-18549-1-git-send-email-vpai@akamai.com>
Hi,
Your patch is applied in the ipset git tree and I'm going to push it for
kernel inclusion.
I modified the comment part: the elements counter can still be incorrect
in the case of a huge set, because elements might time out during the
listing.
Thanks for your patience!
Best regards,
Jozsef
On Thu, 17 Aug 2017, Vishwanath Pai wrote:
> Simple testcase:
>
> $ ipset create test hash:ip timeout 5
> $ ipset add test 1.2.3.4
> $ ipset add test 1.2.2.2
> $ sleep 5
>
> $ ipset l
> Name: test
> Type: hash:ip
> Revision: 5
> Header: family inet hashsize 1024 maxelem 65536 timeout 5
> Size in memory: 296
> References: 0
> Number of entries: 2
> Members:
>
> We return "Number of entries: 2" but no members are listed. That is
> because mtype_list runs "ip_set_timeout_expired" and does not list the
> expired entries, but set->elements is never upated (until mtype_gc
> cleans it up later).
>
> Reviewed-by: Joshua Hunt <johunt@akamai.com>
> Signed-off-by: Vishwanath Pai <vpai@akamai.com>
> ---
> net/netfilter/ipset/ip_set_hash_gen.h | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
> index f236c0b..ff3d31c 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -1041,12 +1041,22 @@ struct htype {
> static int
> mtype_head(struct ip_set *set, struct sk_buff *skb)
> {
> - const struct htype *h = set->data;
> + struct htype *h = set->data;
> const struct htable *t;
> struct nlattr *nested;
> size_t memsize;
> u8 htable_bits;
>
> + /* If any members have expired, set->elements will be wrong
> + * mytype_expire function will update it with the right count.
> + * we do not hold set->lock here, so grab it first.
> + */
> + if (SET_WITH_TIMEOUT(set)) {
> + spin_lock_bh(&set->lock);
> + mtype_expire(set, h);
> + spin_unlock_bh(&set->lock);
> + }
> +
> rcu_read_lock_bh();
> t = rcu_dereference_bh_nfnl(h->table);
> memsize = mtype_ahash_memsize(h, t) + set->ext_size;
> --
> 1.9.1
>
>
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
* Re: [PATCH] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: David Miller @ 2017-09-11 19:34 UTC (permalink / raw)
To: christophe.jaillet-39ZsbGIQGT5GWvitb5QawA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170911192015.17553-1-christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@public.gmane.org>
Please use a proper Subject subsystem prefix for openvswitch patches.
"datapath" isn't very specific in the global kernel namespace at all.
The entire networking stack packet processing path is a "datapath"
"openvswitch: " would have been much better.
^ permalink raw reply
* [PATCH] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Christophe JAILLET @ 2017-09-11 19:20 UTC (permalink / raw)
To: pshelar, davem, xiangxia.m.yue
Cc: netdev, dev, linux-kernel, kernel-janitors, Christophe JAILLET
All other error handling paths in this function go through the 'error'
label. This one should do the same.
Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
I think that the comment above the function could be improved. It looks
like the commit log which has introduced this function.
I'm also not sure that commit 9cc9a5cb176c is of any help. It is
supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
used by each function is reduced, the overall stack should be the same, if
not larger.
So this commit sounds like adding a bug where the code was fine and states
to fix an issue but, at the best, only hides it.
Instead of fixing the code with the proposed patch, reverting the initial
commit could also be considered.
---
net/openvswitch/datapath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 76cf273a56c7..c3aec6227c91 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
"Flow key attribute not present in set flow.");
- return -EINVAL;
+ error = -EINVAL;
+ goto error;
}
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
--
2.11.0
^ permalink raw reply related
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