* Re: [PATCH v2 bpf-next] samples/bpf: xdpsock: order memory on AArch64
From: Björn Töpel @ 2018-07-26 12:18 UTC (permalink / raw)
To: Brian Brooks
Cc: ast, Daniel Borkmann, Björn Töpel, Karlsson, Magnus,
Netdev, LKML
In-Reply-To: <20180725210819.1458-1-brian.brooks@linaro.org>
Den ons 25 juli 2018 kl 23:09 skrev Brian Brooks <brian.brooks@linaro.org>:
>
> Define u_smp_rmb() and u_smp_wmb() to respective barrier instructions.
> This ensures the processor will order accesses to queue indices against
> accesses to queue ring entries.
>
Thanks Brian!
Acked-by: Björn Töpel <bjorn.topel@intel.com>
> Signed-off-by: Brian Brooks <brian.brooks@linaro.org>
> ---
> samples/bpf/xdpsock_user.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index 5904b1543831..1e82f7c617c3 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -145,8 +145,13 @@ static void dump_stats(void);
> } while (0)
>
> #define barrier() __asm__ __volatile__("": : :"memory")
> +#ifdef __aarch64__
> +#define u_smp_rmb() __asm__ __volatile__("dmb ishld": : :"memory")
> +#define u_smp_wmb() __asm__ __volatile__("dmb ishst": : :"memory")
> +#else
> #define u_smp_rmb() barrier()
> #define u_smp_wmb() barrier()
> +#endif
> #define likely(x) __builtin_expect(!!(x), 1)
> #define unlikely(x) __builtin_expect(!!(x), 0)
>
> --
> 2.18.0
>
^ permalink raw reply
* RE: [PATCH RFC/RFT net-next 00/17] net: Convert neighbor tables to per-namespace
From: David Laight @ 2018-07-26 11:12 UTC (permalink / raw)
To: 'Eric W. Biederman', David Ahern
Cc: Cong Wang, David Miller, Linux Kernel Network Developers,
nikita.leshchenko@oracle.com, Roopa Prabhu, Stephen Hemminger,
Ido Schimmel, Jiri Pirko, Saeed Mahameed, Alexander Aring,
linux-wpan@vger.kernel.org, NetFilter, LKML
In-Reply-To: <87zhyfw70m.fsf@xmission.com>
From: Eric W. Biederman
> Sent: 25 July 2018 18:38
...
> >> Further unless I misread something you are increasing the number of
> >> timers to 3 per namespace. If I create create a thousand network
> >> namespaces that feels like it will hurt system performance overall.
> >
> > It seems to me the timers are per neighbor entry not table. The per
> > table ones are for proxies.
>
> It seems I misread that bit when I was refreshing my memory on what
> everything is doing. If we can already have 1024 timers that makes
> timers not a concern.
Surely it is enough to just have a timestamp in each entry.
Deletion of expired items need not be done until insert (which
has the table suitable locked) bumps into an expired item.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] net: fix amd-xgbe flow-control issue
From: Tom Lendacky @ 2018-07-26 13:13 UTC (permalink / raw)
To: tli012345; +Cc: davem, netdev, linux-kernel, tangpengpeng
In-Reply-To: <20180726064516.56740-1-tli012345@163.com>
On 7/26/2018 1:45 AM, tli012345@163.com wrote:
> From: tangpengpeng <tangpengpeng@higon.com>
>
> If we enable or disable xgbe flow-control by ethtool ,
> it does't work.Because the parameter is not properly
> assigned,so we need to adjust the assignment order
> of the parameters.
>
> Signed-off-by: tangpengpeng <tangpengpeng@higon.com>
You should add a Fixes tag as this bug was introduced with commit:
c1ce2f77366b ("amd-xgbe: Fix flow control setting logic")
And since that commit was introduced in 4.2, you'll want to ask David to
push this to stable (4.4 and higher).
With that,
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> drivers/net/ethernet/amd/xgbe/xgbe-mdio.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> index 4b5d625de8f0..8a3a60bb2688 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> @@ -1111,14 +1111,14 @@ static void xgbe_phy_adjust_link(struct xgbe_prv_data *pdata)
>
> if (pdata->tx_pause != pdata->phy.tx_pause) {
> new_state = 1;
> - pdata->hw_if.config_tx_flow_control(pdata);
> pdata->tx_pause = pdata->phy.tx_pause;
> + pdata->hw_if.config_tx_flow_control(pdata);
> }
>
> if (pdata->rx_pause != pdata->phy.rx_pause) {
> new_state = 1;
> - pdata->hw_if.config_rx_flow_control(pdata);
> pdata->rx_pause = pdata->phy.rx_pause;
> + pdata->hw_if.config_rx_flow_control(pdata);
> }
>
> /* Speed support */
>
^ permalink raw reply
* [PATCH net-next] net: hns: Make many functions static
From: YueHaibing @ 2018-07-26 13:19 UTC (permalink / raw)
To: davem, yisen.zhuang, salil.mehta
Cc: linux-kernel, netdev, linyunsheng, matthias.bgg, lipeng321,
YueHaibing
Fixes the following sparse warning:
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:73:20: warning: symbol 'hns_ae_get_handle' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:332:6: warning: symbol 'hns_ae_stop' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:360:6: warning: symbol 'hns_ae_toggle_ring_irq' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:580:6: warning: symbol 'hns_ae_update_stats' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:663:6: warning: symbol 'hns_ae_get_stats' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:695:6: warning: symbol 'hns_ae_get_strings' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:728:5: warning: symbol 'hns_ae_get_sset_count' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:774:6: warning: symbol 'hns_ae_update_led_status' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:786:5: warning: symbol 'hns_ae_cpld_set_led_id' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:798:6: warning: symbol 'hns_ae_get_regs' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:823:5: warning: symbol 'hns_ae_get_regs_len' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c:342:6: warning: symbol 'hns_gmac_update_stats' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c:934:12: warning: symbol 'hns_mac_get_vaddr' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c:953:5: warning: symbol 'hns_mac_get_cfg' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c:343:6: warning: symbol 'hns_dsaf_srst_chns' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c:366:1: warning: symbol 'hns_dsaf_srst_chns_acpi' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c:373:6: warning: symbol 'hns_dsaf_roce_srst' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c:387:6: warning: symbol 'hns_dsaf_roce_srst_acpi' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c:571:5: warning: symbol 'hns_mac_get_sfp_prsnt' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c:589:5: warning: symbol 'hns_mac_get_sfp_prsnt_acpi' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:31:12: warning: symbol 'g_dsaf_mode_match' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:45:5: warning: symbol 'hns_dsaf_get_cfg' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:962:6: warning: symbol 'hns_dsaf_tcam_addr_get' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2087:6: warning: symbol 'hns_dsaf_port_work_rate_cfg' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2837:5: warning: symbol 'hns_dsaf_roce_reset' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c:76:5: warning: symbol 'hns_ppe_common_get_cfg' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c:107:6: warning: symbol 'hns_ppe_common_free_cfg' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c:340:6: warning: symbol 'hns_ppe_uninit_ex' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c:708:5: warning: symbol 'hns_rcb_get_ring_num' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c:744:14: warning: symbol 'hns_rcb_common_get_vaddr' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c:314:6: warning: symbol 'hns_xgmac_update_stats' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_enet.c:1303:6: warning: symbol 'hns_nic_update_stats' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_enet.c:1585:6: warning: symbol 'hns_nic_poll_controller' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_enet.c:1938:6: warning: symbol 'hns_set_multicast_list' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_enet.c:1960:6: warning: symbol 'hns_nic_set_rx_mode' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:661:6: warning: symbol 'hns_get_ringparam' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:811:6: warning: symbol 'hns_get_channels' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:828:6: warning: symbol 'hns_get_ethtool_stats' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:886:6: warning: symbol 'hns_get_strings' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:976:5: warning: symbol 'hns_get_sset_count' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:1010:5: warning: symbol 'hns_phy_led_set' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:1032:5: warning: symbol 'hns_set_phys_id' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:1106:6: warning: symbol 'hns_get_regs' was not declared. Should it be static?
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c | 30 +++++++++++-----------
drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 8 +++---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 14 +++++-----
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 13 +++++-----
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c | 7 ++---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c | 4 +--
.../net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 8 +++---
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 24 +++++++++--------
10 files changed, 60 insertions(+), 52 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
index bd68379..e6aad30 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
@@ -70,8 +70,8 @@ static struct ring_pair_cb *hns_ae_get_ring_pair(struct hnae_queue *q)
return container_of(q, struct ring_pair_cb, q);
}
-struct hnae_handle *hns_ae_get_handle(struct hnae_ae_dev *dev,
- u32 port_id)
+static struct hnae_handle *hns_ae_get_handle(struct hnae_ae_dev *dev,
+ u32 port_id)
{
int vfnum_per_port;
int qnum_per_vf;
@@ -329,7 +329,7 @@ static int hns_ae_start(struct hnae_handle *handle)
return 0;
}
-void hns_ae_stop(struct hnae_handle *handle)
+static void hns_ae_stop(struct hnae_handle *handle)
{
struct hns_mac_cb *mac_cb = hns_get_mac_cb(handle);
@@ -357,7 +357,7 @@ static void hns_ae_reset(struct hnae_handle *handle)
}
}
-void hns_ae_toggle_ring_irq(struct hnae_ring *ring, u32 mask)
+static void hns_ae_toggle_ring_irq(struct hnae_ring *ring, u32 mask)
{
u32 flag;
@@ -577,8 +577,8 @@ static void hns_ae_get_coalesce_range(struct hnae_handle *handle,
*rx_usecs_high = HNS_RCB_RX_USECS_HIGH;
}
-void hns_ae_update_stats(struct hnae_handle *handle,
- struct net_device_stats *net_stats)
+static void hns_ae_update_stats(struct hnae_handle *handle,
+ struct net_device_stats *net_stats)
{
int port;
int idx;
@@ -660,7 +660,7 @@ void hns_ae_update_stats(struct hnae_handle *handle,
net_stats->multicast = mac_cb->hw_stats.rx_mc_pkts;
}
-void hns_ae_get_stats(struct hnae_handle *handle, u64 *data)
+static void hns_ae_get_stats(struct hnae_handle *handle, u64 *data)
{
int idx;
struct hns_mac_cb *mac_cb;
@@ -692,8 +692,8 @@ void hns_ae_get_stats(struct hnae_handle *handle, u64 *data)
hns_dsaf_get_stats(vf_cb->dsaf_dev, p, vf_cb->port_index);
}
-void hns_ae_get_strings(struct hnae_handle *handle,
- u32 stringset, u8 *data)
+static void hns_ae_get_strings(struct hnae_handle *handle,
+ u32 stringset, u8 *data)
{
int port;
int idx;
@@ -725,7 +725,7 @@ void hns_ae_get_strings(struct hnae_handle *handle,
hns_dsaf_get_strings(stringset, p, port, dsaf_dev);
}
-int hns_ae_get_sset_count(struct hnae_handle *handle, int stringset)
+static int hns_ae_get_sset_count(struct hnae_handle *handle, int stringset)
{
u32 sset_count = 0;
struct hns_mac_cb *mac_cb;
@@ -771,7 +771,7 @@ static int hns_ae_config_loopback(struct hnae_handle *handle,
return ret;
}
-void hns_ae_update_led_status(struct hnae_handle *handle)
+static void hns_ae_update_led_status(struct hnae_handle *handle)
{
struct hns_mac_cb *mac_cb;
@@ -783,8 +783,8 @@ void hns_ae_update_led_status(struct hnae_handle *handle)
hns_set_led_opt(mac_cb);
}
-int hns_ae_cpld_set_led_id(struct hnae_handle *handle,
- enum hnae_led_state status)
+static int hns_ae_cpld_set_led_id(struct hnae_handle *handle,
+ enum hnae_led_state status)
{
struct hns_mac_cb *mac_cb;
@@ -795,7 +795,7 @@ int hns_ae_cpld_set_led_id(struct hnae_handle *handle,
return hns_cpld_led_set_id(mac_cb, status);
}
-void hns_ae_get_regs(struct hnae_handle *handle, void *data)
+static void hns_ae_get_regs(struct hnae_handle *handle, void *data)
{
u32 *p = data;
int i;
@@ -820,7 +820,7 @@ void hns_ae_get_regs(struct hnae_handle *handle, void *data)
hns_dsaf_get_regs(vf_cb->dsaf_dev, vf_cb->port_index, p);
}
-int hns_ae_get_regs_len(struct hnae_handle *handle)
+static int hns_ae_get_regs_len(struct hnae_handle *handle)
{
u32 total_num;
struct hnae_vf_cb *vf_cb = hns_ae_get_vf_cb(handle);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
index 74bd260..5488c6e 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
@@ -339,7 +339,7 @@ static void hns_gmac_init(void *mac_drv)
GMAC_TX_WATER_LINE_SHIFT, 8);
}
-void hns_gmac_update_stats(void *mac_drv)
+static void hns_gmac_update_stats(void *mac_drv)
{
struct mac_hw_stats *hw_stats = NULL;
struct mac_driver *drv = (struct mac_driver *)mac_drv;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index 9dcc576..6e5107d 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -931,8 +931,9 @@ static int hns_mac_get_mode(phy_interface_t phy_if)
}
}
-u8 __iomem *hns_mac_get_vaddr(struct dsaf_device *dsaf_dev,
- struct hns_mac_cb *mac_cb, u32 mac_mode_idx)
+static u8 __iomem *
+hns_mac_get_vaddr(struct dsaf_device *dsaf_dev,
+ struct hns_mac_cb *mac_cb, u32 mac_mode_idx)
{
u8 __iomem *base = dsaf_dev->io_base;
int mac_id = mac_cb->mac_id;
@@ -950,7 +951,8 @@ u8 __iomem *hns_mac_get_vaddr(struct dsaf_device *dsaf_dev,
* @mac_cb: mac control block
* return 0 - success , negative --fail
*/
-int hns_mac_get_cfg(struct dsaf_device *dsaf_dev, struct hns_mac_cb *mac_cb)
+static int
+hns_mac_get_cfg(struct dsaf_device *dsaf_dev, struct hns_mac_cb *mac_cb)
{
int ret;
u32 mac_mode_idx;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 0ce07f6..7afc675 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -28,7 +28,7 @@
#include "hns_dsaf_rcb.h"
#include "hns_dsaf_misc.h"
-const char *g_dsaf_mode_match[DSAF_MODE_MAX] = {
+const static char *g_dsaf_mode_match[DSAF_MODE_MAX] = {
[DSAF_MODE_DISABLE_2PORT_64VM] = "2port-64vf",
[DSAF_MODE_DISABLE_6PORT_0VM] = "6port-16rss",
[DSAF_MODE_DISABLE_6PORT_16VM] = "6port-16vf",
@@ -42,7 +42,7 @@ static const struct acpi_device_id hns_dsaf_acpi_match[] = {
};
MODULE_DEVICE_TABLE(acpi, hns_dsaf_acpi_match);
-int hns_dsaf_get_cfg(struct dsaf_device *dsaf_dev)
+static int hns_dsaf_get_cfg(struct dsaf_device *dsaf_dev)
{
int ret, i;
u32 desc_num;
@@ -959,7 +959,8 @@ static void hns_dsaf_tcam_mc_invld(struct dsaf_device *dsaf_dev, u32 address)
spin_unlock_bh(&dsaf_dev->tcam_lock);
}
-void hns_dsaf_tcam_addr_get(struct dsaf_drv_tbl_tcam_key *mac_key, u8 *addr)
+static void
+hns_dsaf_tcam_addr_get(struct dsaf_drv_tbl_tcam_key *mac_key, u8 *addr)
{
addr[0] = mac_key->high.bits.mac_0;
addr[1] = mac_key->high.bits.mac_1;
@@ -2084,8 +2085,9 @@ static void hns_dsaf_pfc_unit_cnt(struct dsaf_device *dsaf_dev, int mac_id,
* @dsaf_id: dsa fabric id
* @xge_ge_work_mode
*/
-void hns_dsaf_port_work_rate_cfg(struct dsaf_device *dsaf_dev, int mac_id,
- enum dsaf_port_rate_mode rate_mode)
+static void
+hns_dsaf_port_work_rate_cfg(struct dsaf_device *dsaf_dev, int mac_id,
+ enum dsaf_port_rate_mode rate_mode)
{
u32 port_work_mode;
@@ -2834,7 +2836,7 @@ module_platform_driver(g_dsaf_driver);
* @enable: false - request reset , true - drop reset
* retuen 0 - success , negative -fail
*/
-int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool dereset)
+static int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool dereset)
{
struct dsaf_device *dsaf_dev;
struct platform_device *pdev;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
index acf2963..16294cd 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
@@ -340,7 +340,8 @@ static void hns_dsaf_xge_srst_by_port_acpi(struct dsaf_device *dsaf_dev,
* bit18-19 for com/dfx
* @enable: false - request reset , true - drop reset
*/
-void hns_dsaf_srst_chns(struct dsaf_device *dsaf_dev, u32 msk, bool dereset)
+static void
+hns_dsaf_srst_chns(struct dsaf_device *dsaf_dev, u32 msk, bool dereset)
{
u32 reg_addr;
@@ -362,7 +363,7 @@ void hns_dsaf_srst_chns(struct dsaf_device *dsaf_dev, u32 msk, bool dereset)
* bit18-19 for com/dfx
* @enable: false - request reset , true - drop reset
*/
-void
+static void
hns_dsaf_srst_chns_acpi(struct dsaf_device *dsaf_dev, u32 msk, bool dereset)
{
hns_dsaf_acpi_srst_by_port(dsaf_dev, HNS_OP_RESET_FUNC,
@@ -370,7 +371,7 @@ hns_dsaf_srst_chns_acpi(struct dsaf_device *dsaf_dev, u32 msk, bool dereset)
msk, dereset);
}
-void hns_dsaf_roce_srst(struct dsaf_device *dsaf_dev, bool dereset)
+static void hns_dsaf_roce_srst(struct dsaf_device *dsaf_dev, bool dereset)
{
if (!dereset) {
dsaf_write_sub(dsaf_dev, DSAF_SUB_SC_ROCEE_RESET_REQ_REG, 1);
@@ -384,7 +385,7 @@ void hns_dsaf_roce_srst(struct dsaf_device *dsaf_dev, bool dereset)
}
}
-void hns_dsaf_roce_srst_acpi(struct dsaf_device *dsaf_dev, bool dereset)
+static void hns_dsaf_roce_srst_acpi(struct dsaf_device *dsaf_dev, bool dereset)
{
hns_dsaf_acpi_srst_by_port(dsaf_dev, HNS_OP_RESET_FUNC,
HNS_ROCE_RESET_FUNC, 0, dereset);
@@ -568,7 +569,7 @@ static phy_interface_t hns_mac_get_phy_if_acpi(struct hns_mac_cb *mac_cb)
return phy_if;
}
-int hns_mac_get_sfp_prsnt(struct hns_mac_cb *mac_cb, int *sfp_prsnt)
+static int hns_mac_get_sfp_prsnt(struct hns_mac_cb *mac_cb, int *sfp_prsnt)
{
u32 val = 0;
int ret;
@@ -586,7 +587,7 @@ int hns_mac_get_sfp_prsnt(struct hns_mac_cb *mac_cb, int *sfp_prsnt)
return 0;
}
-int hns_mac_get_sfp_prsnt_acpi(struct hns_mac_cb *mac_cb, int *sfp_prsnt)
+static int hns_mac_get_sfp_prsnt_acpi(struct hns_mac_cb *mac_cb, int *sfp_prsnt)
{
union acpi_object *obj;
union acpi_object obj_args, argv4;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
index 93e71e2..1c3db67 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
@@ -73,7 +73,7 @@ hns_ppe_common_get_ioaddr(struct ppe_common_cb *ppe_common)
* comm_index: common index
* retuen 0 - success , negative --fail
*/
-int hns_ppe_common_get_cfg(struct dsaf_device *dsaf_dev, int comm_index)
+static int hns_ppe_common_get_cfg(struct dsaf_device *dsaf_dev, int comm_index)
{
struct ppe_common_cb *ppe_common;
int ppe_num;
@@ -104,7 +104,8 @@ int hns_ppe_common_get_cfg(struct dsaf_device *dsaf_dev, int comm_index)
return 0;
}
-void hns_ppe_common_free_cfg(struct dsaf_device *dsaf_dev, u32 comm_index)
+static void
+hns_ppe_common_free_cfg(struct dsaf_device *dsaf_dev, u32 comm_index)
{
dsaf_dev->ppe_common[comm_index] = NULL;
}
@@ -337,7 +338,7 @@ static void hns_ppe_uninit_hw(struct hns_ppe_cb *ppe_cb)
}
}
-void hns_ppe_uninit_ex(struct ppe_common_cb *ppe_common)
+static void hns_ppe_uninit_ex(struct ppe_common_cb *ppe_common)
{
u32 i;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
index e2e2853..9d76e2e 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
@@ -705,7 +705,7 @@ void hns_rcb_get_queue_mode(enum dsaf_mode dsaf_mode, u16 *max_vfn,
}
}
-int hns_rcb_get_ring_num(struct dsaf_device *dsaf_dev)
+static int hns_rcb_get_ring_num(struct dsaf_device *dsaf_dev)
{
switch (dsaf_dev->dsaf_mode) {
case DSAF_MODE_ENABLE_FIX:
@@ -741,7 +741,7 @@ int hns_rcb_get_ring_num(struct dsaf_device *dsaf_dev)
}
}
-void __iomem *hns_rcb_common_get_vaddr(struct rcb_common_cb *rcb_common)
+static void __iomem *hns_rcb_common_get_vaddr(struct rcb_common_cb *rcb_common)
{
struct dsaf_device *dsaf_dev = rcb_common->dsaf_dev;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
index 51e7e9f..40711af 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
@@ -311,7 +311,7 @@ static void hns_xgmac_config_max_frame_length(void *mac_drv, u16 newval)
dsaf_write_dev(drv, XGMAC_MAC_MAX_PKT_SIZE_REG, newval);
}
-void hns_xgmac_update_stats(void *mac_drv)
+static void hns_xgmac_update_stats(void *mac_drv)
{
struct mac_driver *drv = (struct mac_driver *)mac_drv;
struct mac_hw_stats *hw_stats = &drv->mac_cb->hw_stats;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 948b3e0..c2ac187 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1300,7 +1300,7 @@ static int hns_nic_net_set_mac_address(struct net_device *ndev, void *p)
return 0;
}
-void hns_nic_update_stats(struct net_device *netdev)
+static void hns_nic_update_stats(struct net_device *netdev)
{
struct hns_nic_priv *priv = netdev_priv(netdev);
struct hnae_handle *h = priv->ae_handle;
@@ -1582,7 +1582,7 @@ static int hns_nic_do_ioctl(struct net_device *netdev, struct ifreq *ifr,
/* use only for netconsole to poll with the device without interrupt */
#ifdef CONFIG_NET_POLL_CONTROLLER
-void hns_nic_poll_controller(struct net_device *ndev)
+static void hns_nic_poll_controller(struct net_device *ndev)
{
struct hns_nic_priv *priv = netdev_priv(ndev);
unsigned long flags;
@@ -1935,7 +1935,7 @@ static int hns_nic_uc_unsync(struct net_device *netdev,
*
* return void
*/
-void hns_set_multicast_list(struct net_device *ndev)
+static void hns_set_multicast_list(struct net_device *ndev)
{
struct hns_nic_priv *priv = netdev_priv(ndev);
struct hnae_handle *h = priv->ae_handle;
@@ -1957,7 +1957,7 @@ void hns_set_multicast_list(struct net_device *ndev)
}
}
-void hns_nic_set_rx_mode(struct net_device *ndev)
+static void hns_nic_set_rx_mode(struct net_device *ndev)
{
struct hns_nic_priv *priv = netdev_priv(ndev);
struct hnae_handle *h = priv->ae_handle;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 2e14a3a..3957205 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -658,8 +658,8 @@ static void hns_nic_get_drvinfo(struct net_device *net_dev,
* @dev: net device
* @param: ethtool parameter
*/
-void hns_get_ringparam(struct net_device *net_dev,
- struct ethtool_ringparam *param)
+static void hns_get_ringparam(struct net_device *net_dev,
+ struct ethtool_ringparam *param)
{
struct hns_nic_priv *priv = netdev_priv(net_dev);
struct hnae_ae_ops *ops;
@@ -808,7 +808,8 @@ static int hns_set_coalesce(struct net_device *net_dev,
* @dev: net device
* @ch: channel info.
*/
-void hns_get_channels(struct net_device *net_dev, struct ethtool_channels *ch)
+static void
+hns_get_channels(struct net_device *net_dev, struct ethtool_channels *ch)
{
struct hns_nic_priv *priv = netdev_priv(net_dev);
@@ -825,8 +826,8 @@ void hns_get_channels(struct net_device *net_dev, struct ethtool_channels *ch)
* @stats: statistics info.
* @data: statistics data.
*/
-void hns_get_ethtool_stats(struct net_device *netdev,
- struct ethtool_stats *stats, u64 *data)
+static void hns_get_ethtool_stats(struct net_device *netdev,
+ struct ethtool_stats *stats, u64 *data)
{
u64 *p = data;
struct hns_nic_priv *priv = netdev_priv(netdev);
@@ -883,7 +884,7 @@ void hns_get_ethtool_stats(struct net_device *netdev,
* @stats: string set ID.
* @data: objects data.
*/
-void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
+static void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
{
struct hns_nic_priv *priv = netdev_priv(netdev);
struct hnae_handle *h = priv->ae_handle;
@@ -973,7 +974,7 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
*
* Return string set count.
*/
-int hns_get_sset_count(struct net_device *netdev, int stringset)
+static int hns_get_sset_count(struct net_device *netdev, int stringset)
{
struct hns_nic_priv *priv = netdev_priv(netdev);
struct hnae_handle *h = priv->ae_handle;
@@ -1007,7 +1008,7 @@ int hns_get_sset_count(struct net_device *netdev, int stringset)
*
* Return 0 on success, negative on failure.
*/
-int hns_phy_led_set(struct net_device *netdev, int value)
+static int hns_phy_led_set(struct net_device *netdev, int value)
{
int retval;
struct phy_device *phy_dev = netdev->phydev;
@@ -1029,7 +1030,8 @@ int hns_phy_led_set(struct net_device *netdev, int value)
*
* Return 0 on success, negative on failure.
*/
-int hns_set_phys_id(struct net_device *netdev, enum ethtool_phys_id_state state)
+static int
+hns_set_phys_id(struct net_device *netdev, enum ethtool_phys_id_state state)
{
struct hns_nic_priv *priv = netdev_priv(netdev);
struct hnae_handle *h = priv->ae_handle;
@@ -1103,8 +1105,8 @@ int hns_set_phys_id(struct net_device *netdev, enum ethtool_phys_id_state state)
* @cmd: ethtool cmd
* @date: register data
*/
-void hns_get_regs(struct net_device *net_dev, struct ethtool_regs *cmd,
- void *data)
+static void hns_get_regs(struct net_device *net_dev, struct ethtool_regs *cmd,
+ void *data)
{
struct hns_nic_priv *priv = netdev_priv(net_dev);
struct hnae_ae_ops *ops;
--
2.7.0
^ permalink raw reply related
* Re: [PATCH V2 bpf] xdp: add NULL pointer check in __xdp_return()
From: Björn Töpel @ 2018-07-26 12:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Taehee Yoo, Daniel Borkmann, ast, Björn Töpel,
Jesper Dangaard Brouer, Netdev
In-Reply-To: <20180725191149.34242252@cakuba.netronome.com>
Den tors 26 juli 2018 kl 04:14 skrev Jakub Kicinski
<jakub.kicinski@netronome.com>:
>
> On Thu, 26 Jul 2018 00:09:50 +0900, Taehee Yoo wrote:
> > rhashtable_lookup() can return NULL. so that NULL pointer
> > check routine should be added.
> >
> > Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
> > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> > ---
> > V2 : add WARN_ON_ONCE when xa is NULL.
> >
> > net/core/xdp.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 9d1f220..786fdbe 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -345,7 +345,10 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> > rcu_read_lock();
> > /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> > xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > - xa->zc_alloc->free(xa->zc_alloc, handle);
> > + if (!xa)
> > + WARN_ON_ONCE(1);
>
> nit: is compiler smart enough to figure out the fast path here?
> WARN_ON_ONCE() has the nice side effect of wrapping the condition in
> unlikely(). It could save us both LoC and potentially cycles to do:
>
> if (!WARN_ON_ONCE(!xa))
> xa->zc_alloc->free(xa->zc_alloc, handle);
>
> Although it admittedly looks a bit awkward. I'm not sure if we have
> some form of assert (i.e. positive check) in tree :S
>
I'm kind of in favor of this ^^^. Hopefully, Taehee is ok with another spin.
Björn
> > + else
> > + xa->zc_alloc->free(xa->zc_alloc, handle);
> > rcu_read_unlock();
> > default:
> > /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
^ permalink raw reply
* Re: [PATCH bpf] xdp: add NULL pointer check in __xdp_return()
From: Björn Töpel @ 2018-07-26 12:13 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, Björn Töpel, Jesper Dangaard Brouer,
kafai, Taehee Yoo, ast, Netdev, Karlsson, Magnus
In-Reply-To: <20180723125843.7431eb3a@cakuba.netronome.com>
Den mån 23 juli 2018 kl 21:58 skrev Jakub Kicinski
<jakub.kicinski@netronome.com>:
>
> On Mon, 23 Jul 2018 11:39:36 +0200, Björn Töpel wrote:
> > Den fre 20 juli 2018 kl 22:08 skrev Jakub Kicinski:
> > > On Fri, 20 Jul 2018 10:18:21 -0700, Martin KaFai Lau wrote:
> > > > On Sat, Jul 21, 2018 at 01:04:45AM +0900, Taehee Yoo wrote:
> > > > > rhashtable_lookup() can return NULL. so that NULL pointer
> > > > > check routine should be added.
> > > > >
> > > > > Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
> > > > > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> > > > > ---
> > > > > net/core/xdp.c | 3 ++-
> > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > > > > index 9d1f220..1c12bc7 100644
> > > > > --- a/net/core/xdp.c
> > > > > +++ b/net/core/xdp.c
> > > > > @@ -345,7 +345,8 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> > > > > rcu_read_lock();
> > > > > /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> > > > > xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > > > > - xa->zc_alloc->free(xa->zc_alloc, handle);
> > > > > + if (xa)
> > > > > + xa->zc_alloc->free(xa->zc_alloc, handle);
> > > > hmm...It is not clear to me the "!xa" case don't have to be handled?
> > >
> > > Actually I have a more fundamental question about this interface I've
> > > been meaning to ask.
> > >
> > > IIUC free() can happen on any CPU at any time, when whatever device,
> > > socket or CPU this got redirected to completed the TX. IOW there may
> > > be multiple producers. Drivers would need to create spin lock a'la the
> > > a9744f7ca200 ("xsk: fix potential race in SKB TX completion code") fix?
> > >
> >
> > Jakub, apologies for the slow response. I'm still in
> > "holiday/hammock&beer mode", but will be back in a week. :-P
>
> Ah, sorry to interrupt! :)
>
Don't make it a habit! ;-P
> > The idea with the xdp_return_* functions are that an xdp_buff and
> > xdp_frame can have custom allocations schemes. The difference beween
> > struct xdp_buff and struct xdp_frame is lifetime. The xdp_buff
> > lifetime is within the napi context, whereas xdp_frame can have a
> > lifetime longer/outside the napi context. E.g. for a XDP_REDIRECT
> > scenario an xdp_buff is converted to a xdp_frame. The conversion is
> > done in include/net/xdp.h:convert_to_xdp_frame.
> >
> > Currently, the zero-copy MEM_TYPE_ZERO_COPY memtype can *only* be used
> > for xdp_buff, meaning that the lifetime is constrained to a napi
> > context. Further, given an xdp_buff with memtype MEM_TYPE_ZERO_COPY,
> > doing XDP_REDIRECT to a target that is *not* an AF_XDP socket would
> > mean converting the xdp_buff to an xdp_frame. The xdp_frame can then
> > be free'd on any CPU.
> >
> > Note that the xsk_rcv* functions is always called from an napi
> > context, and therefore is using the xdp_return_buff calls.
> >
> > To answer your question -- no, this fix is *not* needed, because the
> > xdp_buff napi constrained, and the xdp_buff will only be free'd on one
> > CPU.
>
> Oh, thanks, I missed the check in convert_to_xdp_frame(), so the only
> frames which can come back via the free path are out of the error path
> in __xsk_rcv_zc()?
>
> That path looks a little surprising too, isn't the expectation that if
> xdp_do_redirect() returns an error the driver retains the ownership of
> the buffer?
>
> static int __xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
> {
> int err = xskq_produce_batch_desc(xs->rx, (u64)xdp->handle, len);
>
> if (err) {
> xdp_return_buff(xdp);
> xs->rx_dropped++;
> }
>
> return err;
> }
>
> This seems to call xdp_return_buff() *and* return an error.
>
Ugh, this is indeed an error. The xdp_return buff should be removed.
Thanks for spotting this!
So, yes, the way to get the buffer back (in ZC) to the driver is via
the error path (recycling) or via the UMEM fill queue.
> > > We need some form of internal kernel circulation which would be MPSC.
> > > I'm currently hacking up the XSK code to tell me whether the frame was
> > > consumed by the correct XSK, and always clone the frame otherwise
> > > (claiming to be the "traditional" MEM_TYPE_PAGE_ORDER0).
> > >
> > > I feel like I'm missing something about the code. Is redirect of
> > > ZC/UMEM frame outside the xsk not possible and the only returns we will
> > > see are from net/xdp/xsk.c? That would work, but I don't see such a
> > > check. Help would be appreciated.
> > >
> >
> > Right now, this is the case (refer to the TODO in
> > convert_to_xdp_frame), i.e. you cannot redirect an ZC/UMEM allocated
> > xdp_buff to a target that is not an xsk. This must, obviously, change
> > so that an xdp_buff (of MEM_TYPE_ZERO_COPY) can be converted to an
> > xdp_frame. The xdp_frame must be able to be free'd from multiple CPUs,
> > so here the a more sophisticated allocation scheme is required.
> >
> > > Also the fact that XSK bufs can't be freed, only completed, adds to the
> > > pain of implementing AF_XDP, we'd certainly need some form of "give
> > > back the frame, but I may need it later" SPSC mechanism, otherwise
> > > driver writers will have tough time. Unless, again, I'm missing
> > > something about the code :)
> > >
> >
> > Yup, moving the recycling scheme from driver to "generic" is a good
> > idea! I need to finish up those i40e zerocopy patches first though...
>
> Interesting, FWIW I wasn't necessarily thinking about full recycling,
> although that would be the holy grail. Just a generic way of giving up
> buffers for example when user changes ring sizes or brings the device
> down.
>
> > (...and I'm very excited that you're doing nfp support for AF_XDP!!!)
>
> Thanks, I'm still way out in the weeds but it's interesting work :)
^ permalink raw reply
* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
From: Jiri Pirko @ 2018-07-26 12:27 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
sridhar.samudrala
In-Reply-To: <20180726100614.GF2222@nanopsycho>
Thu, Jul 26, 2018 at 12:06:14PM CEST, jiri@resnulli.us wrote:
>Thu, Jul 26, 2018 at 09:38:39AM CEST, jiri@resnulli.us wrote:
>>Wed, Jul 25, 2018 at 06:40:44PM CEST, xiyou.wangcong@gmail.com wrote:
>>>On Tue, Jul 24, 2018 at 11:49 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>>>
>>>> Wed, Jul 25, 2018 at 01:20:08AM CEST, xiyou.wangcong@gmail.com wrote:
>>>> >So, you only send out notification when the last refcnt is gone.
>>>> >
>>>> >If the chain that is being deleted by a user is still used by an action,
>>>> >you return 0 or -EPERM?
>>>>
>>>> 0 and the chain stays there until the action is removed. Hmm, do you thing
>>>> that -EPERM should be returned in that case? The thing is, we have to
>>>> flush the chain in order to see the action references are there. We would
>>>> have to have 2 ref counters, one for filter, one for actions.
>>>> What do you think?
>>>
>>>_If_ RTM_DELCHAIN does decrease the chain refcnt, then it is
>>>broken:
>>>
>>># tc chain add X... (refcnt == 1)
>>># tc action add ... goto chain X (refcnt==2)
>>># tc chain del X ... (refcnt== 1)
>>># tc chain del X ... (refcnt==0)
>>>
>>>RTM_DELCHAIN should just test if refcnt is 1, if it is, delete it,
>>>otherwise return -EPERM. This is how we handle tc standalone
>>>actions, see tcf_idr_delete_index().
>>>
>>>Yes, you might need two refcnt's here.
>>
>>Okay. Sounds good. I'm on it.
>
>Actually, I found an issue. The action to "goto chain" might be attached
>to a filter in the same chain. That is completely legitimate usage.
>When I do:
># tc chain del X
>I expect the chain to be flushed and removed. If there is an action
>there with "goto" to the same chain, the command should be successful.
>However, I don't see any easy way to find out if the chain is referenced
>only by actions used by filters in the same chain :/
>
>Thoughts?
I'm now working on a patch that would treat empty chains implicitly
created or deleted by user that only are referenced by action as a
zombie ones. They won't be visible on dump. User won't know about them,
they would only serve as a place holder for "goto chain" actions.
I think it is reasonable. What do you think.
Will send the RFC in few hours.
>
^ permalink raw reply
* Re: [PATCH net-next v3 4/5] net/tc: introduce TC_ACT_REINJECT.
From: Jamal Hadi Salim @ 2018-07-26 12:52 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, Cong Wang
Cc: Paolo Abeni, Linux Kernel Network Developers, Jiri Pirko,
Daniel Borkmann, Eyal Birger, David Miller
In-Reply-To: <20180725170859.GC20383@localhost.localdomain>
On 25/07/18 01:09 PM, Marcelo Ricardo Leitner wrote:
> On Wed, Jul 25, 2018 at 09:48:16AM -0700, Cong Wang wrote:
>> On Wed, Jul 25, 2018 at 5:27 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>>
>>> Those changes were there from the beginning (above patch did
>>> not introduce them).
>>> IIRC, the reason was to distinguish between policy intended
>>> drops and drops because of errors.
>>
>> There must be a limit for "overlimit" to make sense. There is
>> no limit in mirred action's context, probably there is only
>> such a limit in act_police. So, all rest should not touch overlimit.
>
> +1
>
I agree we should at least record drop count(unrelated patch though).
we should keep overlimit (for no other reason other than this
has been around for at least 15 years).
On why "overlimit"? It is just a name for a counter that is useless
for most actions (but was still being transfered to user space).
It is the closest counter to counting "this failed because of
runtime errors" as opposed to "user asked us to drop this".
Probably a good alternative is to make a very small stats v3 structure
(we have migrated stats structures before) and extend for
each action/classifier/qdisc to add its extra counters using XSTATS.
Note:
If you are _mirroring_ packets - incrementing the drop counter is
misleading because the packet is not dropped by the system.
i.e the qdisc will not record it as dropped; it should for
redirect policy. It is useful to be able to tell
them apart when you are collecting analytics just for actions.
(if youve worked on a massive amount of machines you'll appreciate
being able to debug by looking at counters that reduce ambiguity).
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] 9p: fix NULL pointer dereferences
From: Tomas Bortoli @ 2018-07-26 14:13 UTC (permalink / raw)
To: Dominique Martinet, Dmitry Vyukov
Cc: David Miller, v9fs-developer, netdev, LKML, syzkaller
In-Reply-To: <20180726094849.GA18334@nautica>
On 07/26/2018 11:48 AM, Dominique Martinet wrote:
> Dmitry Vyukov wrote on Thu, Jul 26, 2018:
>>> Let's refuse that at much higher level, like v9fs_mount() in
>>> fs/9p/vfs_super.c
>>>
>>> I can't think of any valid reason for dev_name to be NULL, it's the
>>> target IP or virtio handle.
>>
>> But I think trans=fd allows NULL addr today, no?
>
How ?
> Ah, right, I read the patch too fast and read unix_create as fd_create,
> I never realized there was a unix_create variant...
>
> fd legitimately doesn't need a name, you are correct.
>
> I'm really curious if anyone ever uses the unix/fd variants for "real"
> stuff though! (not meaning syzbot isn't real, but I have yet to see
> anything take advantage of this, even if I could imagine some fun
> applications by piping the wmii libixp server socket.. and just crashed
> my laptop trying because of the (fixed) trans put bug.. I have yet to
> see anyone actually doing this)
>
>
> On the other hand, virtio, rdma and xen all have the same problem, so
> Thomas, please fix them instead :)
>
So just by patching v9fs_mount ?
^ permalink raw reply
* Re: net-next boot error
From: Steven Rostedt @ 2018-07-26 14:17 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: syzbot, Borislav Petkov, Jason Baron, Josh Poimboeuf, LKML,
Marc Zyngier, Ingo Molnar, Paolo Bonzini, Peter Zijlstra,
syzkaller-bugs, Michael S. Tsirkin, Jason Wang, David Miller,
virtualization, netdev, Tetsuo Handa, Thomas Gleixner
In-Reply-To: <CACT4Y+ZBf_X=wKQLnAtVz_P-y-6L7+azmpRqT5XFaH5ySx3UiQ@mail.gmail.com>
[ Added Thomas Gleixner ]
On Thu, 26 Jul 2018 11:34:39 +0200
Dmitry Vyukov <dvyukov@google.com> wrote:
> On Thu, Jul 26, 2018 at 11:29 AM, syzbot
> <syzbot+604f8271211546f5b3c7@syzkaller.appspotmail.com> wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: dc66fe43b7eb rds: send: Fix dead code in rds_sendmsg
> > git tree: net-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=127874c8400000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=f34ce142a9f5f0e8
> > dashboard link: https://syzkaller.appspot.com/bug?extid=604f8271211546f5b3c7
> > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> >
> > Unfortunately, I don't have any reproducer for this crash yet.
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+604f8271211546f5b3c7@syzkaller.appspotmail.com
> >
> > possible deadlock in static_key_slow_incsd 0:0:1:0: [sda] Attached SCSI disk
> > MACsec IEEE 802.1AE
> > tun: Universal TUN/TAP device driver, 1.6
> >
> > ============================================
> > WARNING: possible recursive locking detected
>
> +Tetsuo, perhaps this boot lockdep problem then disables lockdep for
> actual testing. I think lockdep should respect panic_on_warn.
>
>
> > 4.18.0-rc6+ #141 Not tainted
> > --------------------------------------------
> > swapper/0/1 is trying to acquire lock:
> > (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at:
> > static_key_slow_inc+0x12/0x30 kernel/jump_label.c:124
> >
> > but task is already holding lock:
> > (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: get_online_cpus
> > include/linux/cpu.h:126 [inline]
> > (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: init_vqs+0xe1a/0x1520
> > drivers/net/virtio_net.c:2777
Here init_vqs() does:
get_online_cpus();
virtnet_set_affinity(vi);
put_online_cpus();
Which disables cpu hotplug and calls virtnet_set_affinity()
Note, get_online_cpus() is no longer recursive.
> >
> > other info that might help us debug this:
> > Possible unsafe locking scenario:
> >
> > CPU0
> > ----
> > lock(cpu_hotplug_lock.rw_sem);
> > lock(cpu_hotplug_lock.rw_sem);
> >
> > *** DEADLOCK ***
> >
> > May be due to missing lock nesting notation
> >
> > 3 locks held by swapper/0/1:
> > #0: (____ptrval____) (&dev->mutex){....}, at: device_lock
> > include/linux/device.h:1134 [inline]
> > #0: (____ptrval____) (&dev->mutex){....}, at: __driver_attach+0x15f/0x2f0
> > drivers/base/dd.c:820
> > #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: get_online_cpus
> > include/linux/cpu.h:126 [inline]
> > #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at:
> > init_vqs+0xe1a/0x1520 drivers/net/virtio_net.c:2777
> > #2: (____ptrval____) (xps_map_mutex){+.+.}, at:
> > __netif_set_xps_queue+0x243/0x23f0 net/core/dev.c:2278
> >
> > stack backtrace:
> > CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.18.0-rc6+ #141
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > Call Trace:
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> > print_deadlock_bug kernel/locking/lockdep.c:1765 [inline]
> > check_deadlock kernel/locking/lockdep.c:1809 [inline]
> > validate_chain kernel/locking/lockdep.c:2405 [inline]
> > __lock_acquire.cold.65+0x1fb/0x486 kernel/locking/lockdep.c:3435
> > lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
> > percpu_down_read_preempt_disable include/linux/percpu-rwsem.h:36 [inline]
> > percpu_down_read include/linux/percpu-rwsem.h:59 [inline]
> > cpus_read_lock+0x43/0xa0 kernel/cpu.c:289
> > static_key_slow_inc+0x12/0x30 kernel/jump_label.c:124
> > __netif_set_xps_queue+0xaac/0x23f0 net/core/dev.c:2320
__netif_set_xps_queue() calls static_key_slow_inc() which will also do
a get_online_cpus() which will trigger this bug.
There's a static_key_slow_inc_cpuslocked() version that should be used
when get_online_cpus() is already taken, but I see
__netif_set_xps_queue() is called from several places, and I doubt it
is always called with get_online_cpus() held. Thus just using the
cpuslocked() version is probably not sufficient of a fix.
I don't know the code enough to offer other suggestions.
-- Steve
> > netif_set_xps_queue+0x26/0x30 net/core/dev.c:2455
> > virtnet_set_affinity+0x2ba/0x4b0 drivers/net/virtio_net.c:1944
> > init_vqs+0xe22/0x1520 drivers/net/virtio_net.c:2778
> > virtnet_probe+0x1092/0x2260 drivers/net/virtio_net.c:3016
> > virtio_dev_probe+0x592/0x942 drivers/virtio/virtio.c:245
> > really_probe drivers/base/dd.c:446 [inline]
> > driver_probe_device+0x6ad/0x970 drivers/base/dd.c:588
> > __driver_attach+0x28b/0x2f0 drivers/base/dd.c:822
> > bus_for_each_dev+0x15d/0x1f0 drivers/base/bus.c:311
> > driver_attach+0x3d/0x50 drivers/base/dd.c:841
> > bus_add_driver+0x4b2/0x600 drivers/base/bus.c:667
> > driver_register+0x1c8/0x320 drivers/base/driver.c:170
> > register_virtio_driver+0x79/0xd0 drivers/virtio/virtio.c:296
> > virtio_net_driver_init+0x8d/0xc9 drivers/net/virtio_net.c:3209
> > do_one_initcall+0x127/0x913 init/main.c:884
> > do_initcall_level init/main.c:952 [inline]
> > do_initcalls init/main.c:960 [inline]
> > do_basic_setup init/main.c:978 [inline]
> > kernel_init_freeable+0x49b/0x58e init/main.c:1135
> > kernel_init+0x11/0x1b3 init/main.c:1061
> > ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> > vcan: Virtual CAN interface driver
> > vxcan: Virtual CAN Tunnel driver
> > slcan: serial line CAN interface driver
> > slcan: 10 dynamic interface channels.
> > CAN device driver interface
> > enic: Cisco VIC Ethernet NIC Driver, ver 2.3.0.53
> > e100: Intel(R) PRO/100 Network Driver, 3.5.24-k2-NAPI
> > e100: Copyright(c) 1999-2006 Intel Corporation
> > e1000: Intel(R) PRO/1000 Network Driver - version 7.3.21-k8-NAPI
> > e1000: Copyright (c) 1999-2006 Intel Corporation.
> > e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k
> > e1000e: Copyright(c) 1999 - 2015 Intel Corporation.
> > sky2: driver version 1.30
> > PPP generic driver version 2.4.2
> > PPP BSD Compression module registered
> > PPP Deflate Compression module registered
> > PPP MPPE Compression module registered
> > NET: Registered protocol family 24
> > PPTP driver version 0.8.5
> > mac80211_hwsim: initializing netlink
> > ieee80211 phy0: Selected rate control algorithm 'minstrel_ht'
> > ieee80211 phy1: Selected rate control algorithm 'minstrel_ht'
> > usbcore: registered new interface driver asix
> > usbcore: registered new interface driver ax88179_178a
> > usbcore: registered new interface driver cdc_ether
> > usbcore: registered new interface driver net1080
> > usbcore: registered new interface driver cdc_subset
> > usbcore: registered new interface driver zaurus
> > usbcore: registered new interface driver cdc_ncm
> > aoe: AoE v85 initialised.
> > ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> > ehci-pci: EHCI PCI platform driver
> > ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
> > ohci-pci: OHCI PCI platform driver
> > uhci_hcd: USB Universal Host Controller Interface driver
> > usbcore: registered new interface driver usblp
> > usbcore: registered new interface driver usb-storage
> > i8042: PNP: PS/2 Controller [PNP0303:KBD,PNP0f13:MOU] at 0x60,0x64 irq 1,12
> > i8042: Warning: Keylock active
> > serio: i8042 KBD port at 0x60,0x64 irq 1
> > serio: i8042 AUX port at 0x60,0x64 irq 12
> > mousedev: PS/2 mouse device common for all mice
> > rtc_cmos 00:00: RTC can wake from S4
> > rtc_cmos 00:00: registered as rtc0
> > rtc_cmos 00:00: alarms up to one day, 114 bytes nvram
> > i2c /dev entries driver
> > piix4_smbus 0000:00:01.3: SMBus base address uninitialized - upgrade BIOS or
> > use force_addr=0xaddr
> > i2c-parport-light: adapter type unspecified
> > usbcore: registered new interface driver RobotFuzz Open Source InterFace,
> > OSIF
> > usbcore: registered new interface driver i2c-tiny-usb
> > device-mapper: ioctl: 4.39.0-ioctl (2018-04-03) initialised:
> > dm-devel@redhat.com
> > device-mapper: raid: Loading target version 1.13.2
> > usbcore: registered new interface driver btusb
> > usnic_verbs: Cisco VIC (USNIC) Verbs Driver v1.0.3 (December 19, 2013)
> > usnic_verbs:usnic_uiom_init:585:
> > IOMMU required but not present or enabled. USNIC QPs will not function w/o
> > enabling IOMMU
> > usnic_verbs:usnic_ib_init:649:
> > Unable to initalize umem with err -1
> > iscsi: registered transport (iser)
> > OPA Virtual Network Driver - v1.0
> > hidraw: raw HID events driver (C) Jiri Kosina
> > usbcore: registered new interface driver usbhid
> > usbhid: USB HID core driver
> > NET: Registered protocol family 40
> > ashmem: initialized
> > NET: Registered protocol family 26
> > Mirror/redirect action on
> > Simple TC action Loaded
> > netem: version 1.3
> > u32 classifier
> > Actions configured
> > nf_conntrack_irc: failed to register helpers
> > nf_conntrack_sane: failed to register helpers
> > nf_conntrack_sip: failed to register helpers
> > xt_time: kernel timezone is -0000
> > IPVS: Registered protocols (TCP, UDP, SCTP, AH, ESP)
> > IPVS: Connection hash table configured (size=4096, memory=64Kbytes)
> > IPVS: ipvs loaded.
> > IPVS: [rr] scheduler registered.
> > IPVS: [wrr] scheduler registered.
> > IPVS: [lc] scheduler registered.
> > IPVS: [wlc] scheduler registered.
> > IPVS: [fo] scheduler registered.
> > IPVS: [ovf] scheduler registered.
> > IPVS: [lblc] scheduler registered.
> > IPVS: [lblcr] scheduler registered.
> > IPVS: [dh] scheduler registered.
> > IPVS: [sh] scheduler registered.
> > IPVS: [mh] scheduler registered.
> > IPVS: [sed] scheduler registered.
> > IPVS: [nq] scheduler registered.
> > IPVS: ftp: loaded support on port[0] = 21
> > IPVS: [sip] pe registered.
> > ipip: IPv4 and MPLS over IPv4 tunneling driver
> > gre: GRE over IPv4 demultiplexor driver
> > ip_gre: GRE over IPv4 tunneling driver
> > IPv4 over IPsec tunneling driver
> > ipt_CLUSTERIP: ClusterIP Version 0.8 loaded successfully
> > Initializing XFRM netlink socket
> > NET: Registered protocol family 10
> > Segment Routing with IPv6
> > mip6: Mobile IPv6
> > sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver
> > ip6_gre: GRE over IPv6 tunneling driver
> > bpfilter: Loaded bpfilter_umh pid 2080
> > NET: Registered protocol family 15
> > Bridge firewalling registered
> > can: controller area network core (rev 20170425 abi 9)
> > NET: Registered protocol family 29
> > can: raw protocol (rev 20170425)
> > can: broadcast manager protocol (rev 20170425 t)
> > can: netlink gateway (rev 20170425) max_hops=1
> > Bluetooth: RFCOMM TTY layer initialized
> > Bluetooth: RFCOMM socket layer initialized
> > Bluetooth: RFCOMM ver 1.11
> > Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> > Bluetooth: BNEP filters: protocol multicast
> > Bluetooth: BNEP socket layer initialized
> > Bluetooth: HIDP (Human Interface Emulation) ver 1.2
> > Bluetooth: HIDP socket layer initialized
> > RPC: Registered rdma transport module.
> > RPC: Registered rdma backchannel transport module.
> > NET: Registered protocol family 41
> > lec:lane_module_init: lec.c: initialized
> > mpoa:atm_mpoa_init: mpc.c: initialized
> > l2tp_core: L2TP core driver, V2.0
> > l2tp_ppp: PPPoL2TP kernel driver, V2.0
> > 8021q: 802.1Q VLAN Support v1.8
> > input: AT Translated Set 2 keyboard as
> > /devices/platform/i8042/serio0/input/input2
> > DCCP: Activated CCID 2 (TCP-like)
> > DCCP: Activated CCID 3 (TCP-Friendly Rate Control)
> > sctp: Hash tables configured (bind 64/64)
> > tipc: Activated (version 2.0.0)
> > NET: Registered protocol family 30
> > tipc: Started in single node mode
> > NET: Registered protocol family 43
> > 9pnet: Installing 9P2000 support
> > NET: Registered protocol family 36
> > Key type dns_resolver registered
> > Key type ceph registered
> > libceph: loaded (mon/osd proto 15/24)
> > openvswitch: Open vSwitch switching datapath
> > mpls_gso: MPLS GSO support
> > start plist test
> > end plist test
> > AVX2 version of gcm_enc/dec engaged.
> > AES CTR mode by8 optimization enabled
> > sched_clock: Marking stable (4559438359, 0)->(6126385605, -1566947246)
> > registered taskstats version 1
> > Loading compiled-in X.509 certificates
> > zswap: default zpool zbud not available
> > zswap: pool creation failed
> > Btrfs loaded, crc32c=crc32c-intel
> > Key type big_key registered
> > Key type encrypted registered
> > Magic number: 10:317:168
> > console [netcon0] enabled
> > netconsole: network logging started
> > gtp: GTP module loaded (pdp ctx size 104 bytes)
> > rdma_rxe: loaded
> > cfg80211: Loading compiled-in X.509 certificates for regulatory database
> > cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
> > platform regulatory.0: Direct firmware load for regulatory.db failed with
> > error -2
> > cfg80211: failed to load regulatory.db
> > ALSA device list:
> > #0: Dummy 1
> > #1: Loopback 1
> > #2: Virtual MIDI Card 1
> > input: ImExPS/2 Generic Explorer Mouse as
> > /devices/platform/i8042/serio1/input/input4
> > md: Waiting for all devices to be available before autodetect
> > md: If you don't use raid, use raid=noautodetect
> > md: Autodetecting RAID arrays.
> > md: autorun ...
> > md: ... autorun DONE.
> > EXT4-fs (sda1): mounted filesystem with ordered data mode. Opts: (null)
> > VFS: Mounted root (ext4 filesystem) readonly on device 8:1.
> > devtmpfs: mounted
> > Freeing unused kernel memory: 3900K
> > Kernel memory protection disabled.
> > SELinux: Disabled at runtime.
> > SELinux: Unregistering netfilter hooks
> > audit: type=1404 audit(1532588961.277:2): enforcing=0 old_enforcing=0
> > auid=4294967295 ses=4294967295 enabled=0 old-enabled=1 lsm=selinux res=1
> > stty (2166) used greatest stack depth: 19664 bytes left
> > EXT4-fs (sda1): re-mounted. Opts: (null)
> > logsave (3615) used greatest stack depth: 17632 bytes left
> > random: dd: uninitialized urandom read (512 bytes read)
> > ==================================================================
> > BUG: KASAN: slab-out-of-bounds in virtnet_receive
> > drivers/net/virtio_net.c:1356 [inline]
>
> +virtio maintainers for this one
> Probably something very recent.
>
> > BUG: KASAN: slab-out-of-bounds in virtnet_poll+0x111a/0x1226
> > drivers/net/virtio_net.c:1421
> > Read of size 8 at addr ffff8801cee08ff0 by task ip/3969
> >
> > CPU: 0 PID: 3969 Comm: ip Not tainted 4.18.0-rc6+ #141
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > Call Trace:
> > <IRQ>
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> > print_address_description+0x6c/0x20b mm/kasan/report.c:256
> > kasan_report_error mm/kasan/report.c:354 [inline]
> > kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
> > __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
> > virtnet_receive drivers/net/virtio_net.c:1356 [inline]
> > virtnet_poll+0x111a/0x1226 drivers/net/virtio_net.c:1421
> > napi_poll net/core/dev.c:6214 [inline]
> > net_rx_action+0x7a5/0x1920 net/core/dev.c:6280
> > __do_softirq+0x2e8/0xb17 kernel/softirq.c:292
> > do_softirq_own_stack+0x2a/0x40 arch/x86/entry/entry_64.S:1046
> > </IRQ>
> > do_softirq.part.18+0x155/0x1a0 kernel/softirq.c:336
> > do_softirq arch/x86/include/asm/preempt.h:23 [inline]
> > __local_bh_enable_ip+0x1ec/0x230 kernel/softirq.c:189
> > local_bh_enable include/linux/bottom_half.h:32 [inline]
> > virtnet_napi_enable+0x8c/0xb0 drivers/net/virtio_net.c:1264
> > virtnet_open+0x16d/0x4d0 drivers/net/virtio_net.c:1464
> > __dev_open+0x26d/0x410 net/core/dev.c:1392
> > __dev_change_flags+0x739/0x9c0 net/core/dev.c:7434
> > dev_change_flags+0x89/0x150 net/core/dev.c:7503
> > do_setlink+0xb16/0x3dd0 net/core/rtnetlink.c:2416
> > rtnl_newlink+0x138d/0x1d60 net/core/rtnetlink.c:3029
> > rtnetlink_rcv_msg+0x46e/0xc30 net/core/rtnetlink.c:4705
> > netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2447
> > rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4723
> > netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
> > netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1336
> > netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1901
> > sock_sendmsg_nosec net/socket.c:641 [inline]
> > sock_sendmsg+0xd5/0x120 net/socket.c:651
> > ___sys_sendmsg+0x7fd/0x930 net/socket.c:2125
> > __sys_sendmsg+0x11d/0x290 net/socket.c:2163
> > __do_sys_sendmsg net/socket.c:2172 [inline]
> > __se_sys_sendmsg net/socket.c:2170 [inline]
> > __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2170
> > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x7f318d594320
> > Code: 02 48 83 c8 ff eb 8d 48 8b 05 14 7b 2a 00 f7 da 64 89 10 48 83 c8 ff
> > eb c9 90 83 3d d5 d2 2a 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff
> > 73 31 c3 48 83 ec 08 e8 5e ba 00 00 48 89 04 24
> > RSP: 002b:00007ffd985d8f38 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > RAX: ffffffffffffffda RBX: 00007ffd985dd030 RCX: 00007f318d594320
> > RDX: 0000000000000000 RSI: 00007ffd985d8f70 RDI: 0000000000000003
> > RBP: 00007ffd985d8f70 R08: 0000000000000000 R09: 000000000000000f
> > R10: 0000000000000000 R11: 0000000000000246 R12: 000000005b5973aa
> > R13: 0000000000000000 R14: 00000000006395c0 R15: 00007ffd985dd808
> >
> > Allocated by task 1:
> > save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> > set_track mm/kasan/kasan.c:460 [inline]
> > kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
> > __do_kmalloc mm/slab.c:3718 [inline]
> > __kmalloc+0x14e/0x760 mm/slab.c:3727
> > kmalloc_array include/linux/slab.h:635 [inline]
> > kcalloc include/linux/slab.h:646 [inline]
> > virtnet_alloc_queues drivers/net/virtio_net.c:2731 [inline]
> > init_vqs+0x127/0x1520 drivers/net/virtio_net.c:2769
> > virtnet_probe+0x1092/0x2260 drivers/net/virtio_net.c:3016
> > virtio_dev_probe+0x592/0x942 drivers/virtio/virtio.c:245
> > really_probe drivers/base/dd.c:446 [inline]
> > driver_probe_device+0x6ad/0x970 drivers/base/dd.c:588
> > __driver_attach+0x28b/0x2f0 drivers/base/dd.c:822
> > bus_for_each_dev+0x15d/0x1f0 drivers/base/bus.c:311
> > driver_attach+0x3d/0x50 drivers/base/dd.c:841
> > bus_add_driver+0x4b2/0x600 drivers/base/bus.c:667
> > driver_register+0x1c8/0x320 drivers/base/driver.c:170
> > register_virtio_driver+0x79/0xd0 drivers/virtio/virtio.c:296
> > virtio_net_driver_init+0x8d/0xc9 drivers/net/virtio_net.c:3209
> > do_one_initcall+0x127/0x913 init/main.c:884
> > do_initcall_level init/main.c:952 [inline]
> > do_initcalls init/main.c:960 [inline]
> > do_basic_setup init/main.c:978 [inline]
> > kernel_init_freeable+0x49b/0x58e init/main.c:1135
> > kernel_init+0x11/0x1b3 init/main.c:1061
> > ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> >
> > Freed by task 0:
> > (stack is not available)
> >
> > The buggy address belongs to the object at ffff8801cee08500
> > which belongs to the cache kmalloc-4096 of size 4096
> > The buggy address is located 2800 bytes inside of
> > 4096-byte region [ffff8801cee08500, ffff8801cee09500)
> > The buggy address belongs to the page:
> > page:ffffea00073b8200 count:1 mapcount:0 mapping:ffff8801dac00dc0 index:0x0
> > compound_mapcount: 0
> > flags: 0x2fffc0000008100(slab|head)
> > raw: 02fffc0000008100 ffffea00073b7d88 ffffea00073b8288 ffff8801dac00dc0
> > raw: 0000000000000000 ffff8801cee08500 0000000100000001 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> > ffff8801cee08e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8801cee08f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >>
> >> ffff8801cee08f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >
> > ^
> > ffff8801cee09000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8801cee09080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ==================================================================
> >
> >
> > ---
> > This bug is generated by a bot. It may contain errors.
> > See https://goo.gl/tpsmEJ for more information about syzbot.
> > syzbot engineers can be reached at syzkaller@googlegroups.com.
> >
> > syzbot will keep track of this bug report. See:
> > https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> > syzbot.
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "syzkaller-bugs" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to syzkaller-bugs+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/syzkaller-bugs/000000000000352dc20571e3a0d8%40google.com.
> > For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH] 9p: fix NULL pointer dereferences
From: Dominique Martinet @ 2018-07-26 14:21 UTC (permalink / raw)
To: Tomas Bortoli
Cc: Dmitry Vyukov, David Miller, v9fs-developer, netdev, LKML,
syzkaller
In-Reply-To: <abbeafa2-9008-5749-aee5-b4a115d71af7@gmail.com>
Tomas Bortoli wrote on Thu, Jul 26, 2018:
>> But I think trans=fd allows NULL addr today, no?
>
> How ?
Just using the mount syscall with a NULL dev_name? I haven't checked
this syzcaller reproducer but it's probably what it does.
p9_fd_create doesn't use 'addr' at all so it's safe to create a 9p mount
for trans=fd with no device name, as Dmitry pointed out
> > On the other hand, virtio, rdma and xen all have the same problem, so
> > Thomas, please fix them instead :)
>
> So just by patching v9fs_mount ?
If we want to preserve the current behaviour for trans=fd (and I don't
see why not) we just have to patch all the transports that use the
device, that is all .create functions but p9_fd_create()
Basically exactly what you did, just for a few more functions - I
apparently was a little bit too optimistic thinking we could share
this check.
--
Dominique
^ permalink raw reply
* Re: [PATCH] 9p: fix NULL pointer dereferences
From: Tomas Bortoli @ 2018-07-26 14:32 UTC (permalink / raw)
To: Dominique Martinet
Cc: Dmitry Vyukov, David Miller, v9fs-developer, netdev, LKML,
syzkaller
In-Reply-To: <20180726142109.GA4235@nautica>
On 07/26/2018 04:21 PM, Dominique Martinet wrote:
> Tomas Bortoli wrote on Thu, Jul 26, 2018:
>>> But I think trans=fd allows NULL addr today, no?
>>
>> How ?
>
> Just using the mount syscall with a NULL dev_name? I haven't checked
> this syzcaller reproducer but it's probably what it does.
>
> p9_fd_create doesn't use 'addr' at all so it's safe to create a 9p mount
> for trans=fd with no device name, as Dmitry pointed out
>
mmh, ok.
>
>>> On the other hand, virtio, rdma and xen all have the same problem, so
>>> Thomas, please fix them instead :)
>>
>> So just by patching v9fs_mount ?
>
> If we want to preserve the current behaviour for trans=fd (and I don't
> see why not) we just have to patch all the transports that use the
> device, that is all .create functions but p9_fd_create()
>
> Basically exactly what you did, just for a few more functions - I
> apparently was a little bit too optimistic thinking we could share
> this check.
>
Does v9fs_mount() knows the transport ahead? Because in that case it'd
be possible to check if addr!=NULL && trans!=fd then return error
Otherwise, patching all the .create, ok.
^ permalink raw reply
* Re: [V9fs-developer] [PATCH] 9p: fix NULL pointer dereferences
From: sqweek @ 2018-07-26 14:48 UTC (permalink / raw)
To: Dominique Martinet
Cc: Dmitry Vyukov, netdev, LKML, syzkaller, V9FS Developers,
Tomas Bortoli, David Miller
In-Reply-To: <20180726094849.GA18334@nautica>
[-- Attachment #1: Type: text/plain, Size: 1688 bytes --]
On 26 July 2018 at 17:48, Dominique Martinet <asmadeus@codewreck.org> wrote:
> Dmitry Vyukov wrote on Thu, Jul 26, 2018:
> > > Let's refuse that at much higher level, like v9fs_mount() in
> > > fs/9p/vfs_super.c
> > >
> > > I can't think of any valid reason for dev_name to be NULL, it's the
> > > target IP or virtio handle.
> >
> > But I think trans=fd allows NULL addr today, no?
>
> Ah, right, I read the patch too fast and read unix_create as fd_create,
> I never realized there was a unix_create variant...
>
> fd legitimately doesn't need a name, you are correct.
>
> I'm really curious if anyone ever uses the unix/fd variants for "real"
> stuff though!
I definitely used the unix variant for mounting plan9port servers (which
all listen for 9p requests via unix sockets).
A long time ago I also experimented with mounting p9p servers from remote
machines and I think I might have combined socat with -o trans=fd at one
point. But I gave up on it in the end because having a process blocked in
read() was preventing my laptop from going to sleep for the duration. And
since I was trying to read /event type files the Tread could block for
quite some time if no events were posted.
I believe 9pfuse had the same issue, so the problem be deeper than v9fs.
But anyway I'm on quite a tangent already so I'll stop distracting you all
from the work at hand. It's been nice to see 9p still kicking recently! :)
> (not meaning syzbot isn't real, but I have yet to see
> anything take advantage of this, even if I could imagine some fun
> applications by piping the wmii libixp server socket..
>
Wait woahhhhh, there's another wmii user left? I thought I was the last one!
-sqweek
[-- Attachment #2: Type: text/html, Size: 2410 bytes --]
^ permalink raw reply
* Re: [RFC PATCH ghak90 (was ghak32) V3 07/10] audit: add support for containerid to network namespaces
From: Richard Guy Briggs @ 2018-07-26 13:33 UTC (permalink / raw)
To: Paul Moore
Cc: cgroups, containers, linux-api, linux-audit, linux-fsdevel,
linux-kernel, netdev, ebiederm, luto, jlayton, carlos, dhowells,
viro, simo, Eric Paris, serge
In-Reply-To: <CAHC9VhSuczaFi49-3SrVM9vND=vT=U1Aj0G6kOyJ3rAFnvi6VA@mail.gmail.com>
On 2018-07-24 16:33, Paul Moore wrote:
> On Tue, Jul 24, 2018 at 10:06 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2018-07-20 18:14, Paul Moore wrote:
> > > On Wed, Jun 6, 2018 at 1:03 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > Audit events could happen in a network namespace outside of a task
> > > > context due to packets received from the net that trigger an auditing
> > > > rule prior to being associated with a running task. The network
> > > > namespace could in use by multiple containers by association to the
> > > > tasks in that network namespace. We still want a way to attribute
> > > > these events to any potential containers. Keep a list per network
> > > > namespace to track these audit container identifiiers.
> > > >
> > > > Add/increment the audit container identifier on:
> > > > - initial setting of the audit container identifier via /proc
> > > > - clone/fork call that inherits an audit container identifier
> > > > - unshare call that inherits an audit container identifier
> > > > - setns call that inherits an audit container identifier
> > > > Delete/decrement the audit container identifier on:
> > > > - an inherited audit container identifier dropped when child set
> > > > - process exit
> > > > - unshare call that drops a net namespace
> > > > - setns call that drops a net namespace
> > > >
> > > > See: https://github.com/linux-audit/audit-kernel/issues/92
> > > > See: https://github.com/linux-audit/audit-testsuite/issues/64
> > > > See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > ---
> > > > include/linux/audit.h | 23 ++++++++++++++++
> > > > kernel/audit.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > kernel/auditsc.c | 5 ++++
> > > > kernel/nsproxy.c | 4 +++
> > > > 4 files changed, 104 insertions(+)
>
> ...
>
> > > > + }
> > > > + if (!found)
> > > > + return;
> > > > + list_del(&cont->list);
> > > > + if (refcount_dec_and_test(&cont->refcount))
> > > > + kfree(cont);
> > >
> > > Don't you want to dec_and_test first and only remove it from the list
> > > if there are no other references?
> >
> > I don't think so. Let me try to describe it in prose to see if I
> > understood this properly and see if this makes more sense: I want to
> > remove this audit_contid list member from this net's audit_contid list
> > and decrement unconditionally this member's refcount so it knows there
> > is one less thing pointing at it and when there is no longer anything
> > pointing at it, free it.
>
> Yep, sorry, my mistake, I was thinking the other way around (netns
> going away) ... which actually, this patchset doesn't handle that does
> it (I don't see any new code in audit_net_exit())? Is is in a later
> patch? If so, it really should be in the same patch as this code to
> prevent bisect nasties.
I don't think any code is needed in audit_net_exit() other than a WARN()
that the contid list is empty, since if a netns is going away, all its
tasks would have exited (removing its contid from the list) or moved to
a new nstns (moving its contid from this list to another netns).
At worst, this is a memory leak and not a bad pointer dereference.
> paul moore
- RGB
^ permalink raw reply
* [PATCH iproute2-next 1/1] tc: fix bugs for tcp_flags and ip_attr hex output
From: Keara Leibovitz @ 2018-07-26 13:45 UTC (permalink / raw)
To: dsahern; +Cc: stephen, netdev, kernel, Keara Leibovitz
Fix hex output for both the ip_attr and tcp_flags print functions.
Sample usage:
$ $TC qdisc add dev lo ingress
$ $TC filter add dev lo parent ffff: prio 3 proto ip flower ip_tos 0x8/32
$ $TC fitler add dev lo parent ffff: prio 5 proto ip flower ip_proto tcp \
tcp_flags 0x909/f00
$ $TC filter show dev lo parent ffff:
filter protocol ip pref 3 flower chain 0
filter protocol ip pref 3 flower chain 0 handle 0x1
eth_type ipv4
ip_tos 0x8/32
not_in_hw
filter protocol ip pref 5 flower chain 0
filter protocol ip pref 5 flower chain 0 handle 0x1
eth_type ipv4
ip_proto tcp
tcp_flags 0x909/f00
not_in_hw
$ $TC -j filter show dev lo parent ffff:
[{
"protocol":"ip",
"pref":3,
"kind":"flower",
"chain":0
},{
"protocol":"ip",
"pref":3,
"kind":"flower",
"chain":0,
"options": {
"handle":1,
"keys": {
"eth_type":"ipv4",
"ip_tos":"0x8/32"
},
"not_in_hw":true
}
},{
"protocol":"ip",
"pref":5,
"kind":"flower",
"chain":0
},{
"protocol":"ip",
"pref":5,
"kind":"flower",
"chain":0,
"options": {
"handle":1,
"keys": {
"eth_type":"ipv4",
"ip_proto":"tcp",
"tcp_flags":"0x909/f00"
},
"not_in_hw":true
}
}]
Signed-off-by: Keara Leibovitz <kleib@mojatatu.com>
---
tc/f_flower.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 9a3fd7751630..59e5f572c542 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1189,7 +1189,7 @@ static void flower_print_ip_proto(__u8 *p_ip_proto,
*p_ip_proto = ip_proto;
}
-static void flower_print_ip_attr(char *name, struct rtattr *key_attr,
+static void flower_print_ip_attr(const char *name, struct rtattr *key_attr,
struct rtattr *mask_attr)
{
SPRINT_BUF(namefrm);
@@ -1199,11 +1199,12 @@ static void flower_print_ip_attr(char *name, struct rtattr *key_attr,
if (!key_attr)
return;
- done = sprintf(out, "%x", rta_getattr_u8(key_attr));
+ done = sprintf(out, "0x%x", rta_getattr_u8(key_attr));
if (mask_attr)
sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
- sprintf(namefrm, "\n %s %%s", name);
+ print_string(PRINT_FP, NULL, "%s ", _SL_);
+ sprintf(namefrm, "%s %%s", name);
print_string(PRINT_ANY, name, namefrm, out);
}
@@ -1308,7 +1309,7 @@ static void flower_print_port(char *name, struct rtattr *attr)
print_hu(PRINT_ANY, name, namefrm, rta_getattr_be16(attr));
}
-static void flower_print_tcp_flags(char *name, struct rtattr *flags_attr,
+static void flower_print_tcp_flags(const char *name, struct rtattr *flags_attr,
struct rtattr *mask_attr)
{
SPRINT_BUF(namefrm);
@@ -1318,11 +1319,12 @@ static void flower_print_tcp_flags(char *name, struct rtattr *flags_attr,
if (!flags_attr)
return;
- done = sprintf(out, "%x", rta_getattr_be16(flags_attr));
+ done = sprintf(out, "0x%x", rta_getattr_be16(flags_attr));
if (mask_attr)
- sprintf(out + done, "%x", rta_getattr_be16(flags_attr));
+ sprintf(out + done, "/%x", rta_getattr_be16(mask_attr));
- sprintf(namefrm, "\n %s %%s", name);
+ print_string(PRINT_FP, NULL, "%s ", _SL_);
+ sprintf(namefrm, "%s %%s", name);
print_string(PRINT_ANY, name, namefrm, out);
}
--
2.7.4
^ permalink raw reply related
* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Alexander Duyck @ 2018-07-26 14:00 UTC (permalink / raw)
To: Jiri Pirko
Cc: Jakub Kicinski, Eran Ben Elisha, Saeed Mahameed, David S. Miller,
netdev@vger.kernel.org, linux-pci
In-Reply-To: <20180726071433.GA2222@nanopsycho>
On Thu, Jul 26, 2018 at 12:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Jul 26, 2018 at 02:43:59AM CEST, jakub.kicinski@netronome.com wrote:
>>On Wed, 25 Jul 2018 08:23:26 -0700, Alexander Duyck wrote:
>>> On Wed, Jul 25, 2018 at 5:31 AM, Eran Ben Elisha wrote:
>>> > On 7/24/2018 10:51 PM, Jakub Kicinski wrote:
>>> >>>> The devlink params haven't been upstream even for a full cycle and
>>> >>>> already you guys are starting to use them to configure standard
>>> >>>> features like queuing.
>>> >>>
>>> >>> We developed the devlink params in order to support non-standard
>>> >>> configuration only. And for non-standard, there are generic and vendor
>>> >>> specific options.
>>> >>
>>> >> I thought it was developed for performing non-standard and possibly
>>> >> vendor specific configuration. Look at DEVLINK_PARAM_GENERIC_* for
>>> >> examples of well justified generic options for which we have no
>>> >> other API. The vendor mlx4 options look fairly vendor specific if you
>>> >> ask me, too.
>>> >>
>>> >> Configuring queuing has an API. The question is it acceptable to enter
>>> >> into the risky territory of controlling offloads via devlink parameters
>>> >> or would we rather make vendors take the time and effort to model
>>> >> things to (a subset) of existing APIs. The HW never fits the APIs
>>> >> perfectly.
>>> >
>>> > I understand what you meant here, I would like to highlight that this
>>> > mechanism was not meant to handle SRIOV, Representors, etc.
>>> > The vendor specific configuration suggested here is to handle a congestion
>>> > state in Multi Host environment (which includes PF and multiple VFs per
>>> > host), where one host is not aware to the other hosts, and each is running
>>> > on its own pci/driver. It is a device working mode configuration.
>>> >
>>> > This couldn't fit into any existing API, thus creating this vendor specific
>>> > unique API is needed.
>>>
>>> If we are just going to start creating devlink interfaces in for every
>>> one-off option a device wants to add why did we even bother with
>>> trying to prevent drivers from using sysfs? This just feels like we
>>> are back to the same arguments we had back in the day with it.
>>>
>>> I feel like the bigger question here is if devlink is how we are going
>>> to deal with all PCIe related features going forward, or should we
>>> start looking at creating a new interface/tool for PCI/PCIe related
>>> features? My concern is that we have already had features such as DMA
>>> Coalescing that didn't really fit into anything and now we are
>>> starting to see other things related to DMA and PCIe bus credits. I'm
>>> wondering if we shouldn't start looking at a tool/interface to
>>> configure all the PCIe related features such as interrupts, error
>>> reporting, DMA configuration, power management, etc. Maybe we could
>>> even look at sharing it across subsystems and include things like
>>> storage, graphics, and other subsystems in the conversation.
>>
>>Agreed, for actual PCIe configuration (i.e. not ECN marking) we do need
>>to build up an API. Sharing it across subsystems would be very cool!
>
> I wonder howcome there isn't such API in place already. Or is it?
> If it is not, do you have any idea how should it look like? Should it be
> an extension of the existing PCI uapi or something completely new?
> It would be probably good to loop some PCI people in...
The closest thing I can think of in terms of answering your questions
as to why we haven't seen anything like that would be setpci.
Basically with that tool you can go through the PCI configuration
space and update any piece you want. The problem is it can have
effects on the driver and I don't recall there ever being any sort of
notification mechanism added to make a driver aware of configuration
updates.
As far as the interface I don't know if we would want to use something
like netlink or look at something completely new.
I've gone ahead and added the linux-pci mailing list to the thread.
- Alex
^ permalink raw reply
* Re: [PATCH V2 bpf] xdp: add NULL pointer check in __xdp_return()
From: Taehee Yoo @ 2018-07-26 14:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, ast, Björn Töpel,
Jesper Dangaard Brouer, Netdev
In-Reply-To: <20180725191149.34242252@cakuba.netronome.com>
2018-07-26 11:11 GMT+09:00 Jakub Kicinski <jakub.kicinski@netronome.com>:
> On Thu, 26 Jul 2018 00:09:50 +0900, Taehee Yoo wrote:
>> rhashtable_lookup() can return NULL. so that NULL pointer
>> check routine should be added.
>>
>> Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
>> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
>> ---
>> V2 : add WARN_ON_ONCE when xa is NULL.
>>
>> net/core/xdp.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 9d1f220..786fdbe 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -345,7 +345,10 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
>> rcu_read_lock();
>> /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
>> xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
>> - xa->zc_alloc->free(xa->zc_alloc, handle);
>> + if (!xa)
>> + WARN_ON_ONCE(1);
>
> nit: is compiler smart enough to figure out the fast path here?
> WARN_ON_ONCE() has the nice side effect of wrapping the condition in
> unlikely(). It could save us both LoC and potentially cycles to do:
>
> if (!WARN_ON_ONCE(!xa))
> xa->zc_alloc->free(xa->zc_alloc, handle);
>
> Although it admittedly looks a bit awkward. I'm not sure if we have
> some form of assert (i.e. positive check) in tree :S
>
Thank you for suggestion!
I like this code style and I think there is no problem because readers
are familiar with this code style.
I will send v3 patch!
Thanks!
>> + else
>> + xa->zc_alloc->free(xa->zc_alloc, handle);
>> rcu_read_unlock();
>> default:
>> /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
^ permalink raw reply
* Re: [PATCH V2 bpf] xdp: add NULL pointer check in __xdp_return()
From: Taehee Yoo @ 2018-07-26 14:05 UTC (permalink / raw)
To: Björn Töpel
Cc: Jakub Kicinski, Daniel Borkmann, ast, Björn Töpel,
Jesper Dangaard Brouer, Netdev
In-Reply-To: <CAJ+HfNhk9iM4av=s-PsX9N=htMUV6um5V-OoC6Y3PO+5vkX7wA@mail.gmail.com>
2018-07-26 21:07 GMT+09:00 Björn Töpel <bjorn.topel@gmail.com>:
> Den tors 26 juli 2018 kl 04:14 skrev Jakub Kicinski
> <jakub.kicinski@netronome.com>:
>>
>> On Thu, 26 Jul 2018 00:09:50 +0900, Taehee Yoo wrote:
>> > rhashtable_lookup() can return NULL. so that NULL pointer
>> > check routine should be added.
>> >
>> > Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
>> > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
>> > ---
>> > V2 : add WARN_ON_ONCE when xa is NULL.
>> >
>> > net/core/xdp.c | 5 ++++-
>> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/net/core/xdp.c b/net/core/xdp.c
>> > index 9d1f220..786fdbe 100644
>> > --- a/net/core/xdp.c
>> > +++ b/net/core/xdp.c
>> > @@ -345,7 +345,10 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
>> > rcu_read_lock();
>> > /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
>> > xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
>> > - xa->zc_alloc->free(xa->zc_alloc, handle);
>> > + if (!xa)
>> > + WARN_ON_ONCE(1);
>>
>> nit: is compiler smart enough to figure out the fast path here?
>> WARN_ON_ONCE() has the nice side effect of wrapping the condition in
>> unlikely(). It could save us both LoC and potentially cycles to do:
>>
>> if (!WARN_ON_ONCE(!xa))
>> xa->zc_alloc->free(xa->zc_alloc, handle);
>>
>> Although it admittedly looks a bit awkward. I'm not sure if we have
>> some form of assert (i.e. positive check) in tree :S
>>
>
> I'm kind of in favor of this ^^^. Hopefully, Taehee is ok with another spin.
>
I like this code style and I think it has performance benefit.
So I will send v3 patch!
Thanks!
> Björn
>
>> > + else
>> > + xa->zc_alloc->free(xa->zc_alloc, handle);
>> > rcu_read_unlock();
>> > default:
>> > /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
^ permalink raw reply
* Re: [Query]: DSA Understanding
From: Andrew Lunn @ 2018-07-26 14:08 UTC (permalink / raw)
To: Lad, Prabhakar; +Cc: netdev
In-Reply-To: <CA+V-a8vdEicXXN_yiRgBfptHx5=_RgX3VjfqiOv+UQY3G3+9oQ@mail.gmail.com>
> Yes I am using fixed phy on slave1, following is my dts:
Posting the original DTS file is better, not the decompiled version.
>
> ethernet@48484000 {
> compatible = "ti,dra7-cpsw", "ti,cpsw";
> ti,hwmods = "gmac";
> clocks = <0x124 0x125>;
> clock-names = "fck", "cpts";
> cpdma_channels = <0x8>;
> ale_entries = <0x400>;
> bd_ram_size = <0x2000>;
> mac_control = <0x20>;
> slaves = <0x2>;
> active_slave = <0x0>;
> cpts_clock_mult = <0x784cfe14>;
> cpts_clock_shift = <0x1d>;
> reg = <0x48484000 0x1000 0x48485200 0x2e00>;
> #address-cells = <0x1>;
> #size-cells = <0x1>;
> ti,no-idle;
> interrupts = <0x0 0x14e 0x4 0x0 0x14f 0x4 0x0 0x150 0x4
> 0x0 0x151 0x4>;
> ranges;
> syscon = <0x8>;
> status = "okay";
> pinctrl-names = "default", "sleep";
> pinctrl-0 = <0x126 0x127>;
> pinctrl-1 = <0x128 0x129>;
> dual_emac;
> linux,phandle = <0x500>;
> phandle = <0x500>;
So here is 0x500
>
> mdio@48485000 {
> compatible = "ti,cpsw-mdio", "ti,davinci_mdio";
> #address-cells = <0x1>;
> #size-cells = <0x0>;
> ti,hwmods = "davinci_mdio";
> bus_freq = <0xf4240>;
> reg = <0x48485000 0x100>;
> status = "okay";
> pinctrl-names = "default", "sleep";
> pinctrl-0 = <0x12a>;
> pinctrl-1 = <0x12b>;
>
> ethernet-phy@1 {
> reg = <0x1>;
> linux,phandle = <0x12c>;
> phandle = <0x12c>;
> };
> };
>
> slave@48480200 {
> mac-address = [00 00 00 00 00 00];
> status = "okay";
> phy-handle = <0x12c>;
> phy-mode = "rgmii";
> dual_emac_res_vlan = <0x1>;
> };
>
> slave@48480300 {
> mac-address = [00 00 00 00 00 00];
> status = "okay";
> phy-mode = "rgmii";
> dual_emac_res_vlan = <0x2>;
> linux,phandle = <0xf3>;
> phandle = <0xf3>;
This is the actual interface you are using and it has a phandle of
0xf3.
>
> fixed-link {
> speed = <0x3e8>;
> full-duplex;
> };
> };
>
> cpsw-phy-sel@4a002554 {
> compatible = "ti,dra7xx-cpsw-phy-sel";
> reg = <0x4a002554 0x4>;
> reg-names = "gmii-sel";
> };
> };
>
> spi@480ba000 {
> compatible = "ti,omap4-mcspi";
> reg = <0x480ba000 0x200>;
> interrupts = <0x0 0x2b 0x4>;
> #address-cells = <0x1>;
> #size-cells = <0x0>;
> ti,hwmods = "mcspi4";
> ti,spi-num-cs = <0x1>;
> dmas = <0xb2 0x46 0xb2 0x47>;
> dma-names = "tx0", "rx0";
> status = "okay";
> ti,pindir-d0-out-d1-in;
>
> ksz9477@0 {
> compatible = "microchip,ksz9477";
> reg = <0x0>;
> spi-max-frequency = <0x2dc6c00>;
> spi-cpha;
> spi-cpol;
>
> ports {
> #address-cells = <0x1>;
> #size-cells = <0x0>;
>
> port@0 {
> reg = <0x0>;
> label = "lan0";
> };
>
> port@1 {
> reg = <0x1>;
> label = "lan1";
> };
>
> port@2 {
> reg = <0x2>;
> label = "lan2";
> };
>
> port@3 {
> reg = <0x3>;
> label = "lan3";
> };
>
> port@4 {
> reg = <0x4>;
> label = "lan4";
> };
>
> port@5 {
> reg = <0x5>;
> label = "cpu";
> ethernet = <0x500>;
So here you are pointing to the container node, not the actual
interface. You talk of having to patch DSA. That i would say is
wrong. You should be using the phandle of the actual interface.
> };
>
> port@6 {
> reg = <0x6>;
> label = "lan6";
> };
> };
> };
> };
>
> Where port 6 of ksz9897 chip is connected to slave 1 (eth1) of cpsw
> via rgmii interface. Following is the dmesg log:
>
> [ 2.596958] Microchip KSZ9477 dsa-0.0:00: attached PHY driver
> [Microchip KSZ9477] (mii_bus:phy_addr=dsa-0.0:00, irq=POLL)
> [ 2.714959] Microchip KSZ9477 dsa-0.0:01: attached PHY driver
> [Microchip KSZ9477] (mii_bus:phy_addr=dsa-0.0:01, irq=POLL)
> [ 2.832958] Microchip KSZ9477 dsa-0.0:02: attached PHY driver
> [Microchip KSZ9477] (mii_bus:phy_addr=dsa-0.0:02, irq=POLL)
> [ 2.950960] Microchip KSZ9477 dsa-0.0:03: attached PHY driver
> [Microchip KSZ9477] (mii_bus:phy_addr=dsa-0.0:03, irq=POLL)
> [ 3.068958] Microchip KSZ9477 dsa-0.0:04: attached PHY driver
> [Microchip KSZ9477] (mii_bus:phy_addr=dsa-0.0:04, irq=POLL)
> [ 3.080953] Generic PHY dsa-0.0:06: attached PHY driver [Generic
> PHY] (mii_bus:phy_addr=dsa-0.0:06, irq=POLL)
You might also need a fixed-link for the cpu port of the switch.
What do the statistics counters show? Is it trying to send/receiver
frames out eth1? Are bad frames received? Maybe you need to set rgmii
delays?
Andrew
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the bpf tree
From: Martin KaFai Lau @ 2018-07-26 15:32 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, Networking, Linux-Next Mailing List,
Linux Kernel Mailing List, Okash Khawaja, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20180726111916.47a47dbf@canb.auug.org.au>
On Thu, Jul 26, 2018 at 11:19:16AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got conflicts in:
>
> tools/lib/bpf/btf.c
> tools/lib/bpf/btf.h
>
> between commits:
>
> 5b891af7fca1 ("bpf: Replace [u]int32_t and [u]int64_t in libbpf")
> 38d5d3b3d5db ("bpf: Introduce BPF_ANNOTATE_KV_PAIR")
>
> from the bpf tree and commit:
>
> 92b57121ca79 ("bpf: btf: export btf types and name by offset from lib")
>
> from the net-next tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
The fix LGTM. Thanks!
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc tools/lib/bpf/btf.c
> index 2d270c560df3,03161be094b4..000000000000
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@@ -269,9 -259,29 +266,29 @@@ __s64 btf__resolve_size(const struct bt
> return nelems * size;
> }
>
> + int btf__resolve_type(const struct btf *btf, __u32 type_id)
> + {
> + const struct btf_type *t;
> + int depth = 0;
> +
> + t = btf__type_by_id(btf, type_id);
> + while (depth < MAX_RESOLVE_DEPTH &&
> + !btf_type_is_void_or_null(t) &&
> + IS_MODIFIER(BTF_INFO_KIND(t->info))) {
> + type_id = t->type;
> + t = btf__type_by_id(btf, type_id);
> + depth++;
> + }
> +
> + if (depth == MAX_RESOLVE_DEPTH || btf_type_is_void_or_null(t))
> + return -EINVAL;
> +
> + return type_id;
> + }
> +
> -int32_t btf__find_by_name(const struct btf *btf, const char *type_name)
> +__s32 btf__find_by_name(const struct btf *btf, const char *type_name)
> {
> - uint32_t i;
> + __u32 i;
>
> if (!strcmp(type_name, "void"))
> return 0;
> @@@ -368,3 -379,20 +385,11 @@@ int btf__fd(const struct btf *btf
> {
> return btf->fd;
> }
> +
> + const char *btf__name_by_offset(const struct btf *btf, __u32 offset)
> + {
> + if (offset < btf->hdr->str_len)
> + return &btf->strings[offset];
> + else
> + return NULL;
> + }
> -
> -const struct btf_type *btf__type_by_id(const struct btf *btf,
> - __u32 type_id)
> -{
> - if (type_id > btf->nr_types)
> - return NULL;
> -
> - return btf->types[type_id];
> -}
> diff --cc tools/lib/bpf/btf.h
> index e2a09a155f84,24f361d99a5e..000000000000
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@@ -15,10 -14,12 +15,12 @@@ typedef int (*btf_print_fn_t)(const cha
> __attribute__((format(printf, 1, 2)));
>
> void btf__free(struct btf *btf);
> -struct btf *btf__new(uint8_t *data, uint32_t size, btf_print_fn_t err_log);
> -int32_t btf__find_by_name(const struct btf *btf, const char *type_name);
> -int64_t btf__resolve_size(const struct btf *btf, uint32_t type_id);
> +struct btf *btf__new(__u8 *data, __u32 size, btf_print_fn_t err_log);
> +__s32 btf__find_by_name(const struct btf *btf, const char *type_name);
> - const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 id);
> +__s64 btf__resolve_size(const struct btf *btf, __u32 type_id);
> + int btf__resolve_type(const struct btf *btf, __u32 type_id);
> int btf__fd(const struct btf *btf);
> + const char *btf__name_by_offset(const struct btf *btf, __u32 offset);
> + const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 type_id);
>
> #endif
^ permalink raw reply
* [PATCH V3 bpf] xdp: add NULL pointer check in __xdp_return()
From: Taehee Yoo @ 2018-07-26 14:17 UTC (permalink / raw)
To: daniel, ast
Cc: netdev, bjorn.topel, brouer, kafai, jakub.kicinski, Taehee Yoo
rhashtable_lookup() can return NULL. so that NULL pointer
check routine should be added.
Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
V3 : reduce code line
V2 : add WARN_ON_ONCE when xa is NULL.
net/core/xdp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 9d1f220..6771f18 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -345,7 +345,8 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
rcu_read_lock();
/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
- xa->zc_alloc->free(xa->zc_alloc, handle);
+ if (!WARN_ON_ONCE(!xa))
+ xa->zc_alloc->free(xa->zc_alloc, handle);
rcu_read_unlock();
default:
/* Not possible, checked in xdp_rxq_info_reg_mem_model() */
--
2.9.3
^ permalink raw reply related
* [PATCH v4 bpf-next 0/9] veth: Driver XDP
From: Toshiaki Makita @ 2018-07-26 14:25 UTC (permalink / raw)
To: netdev, Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, Jesper Dangaard Brouer, Jakub Kicinski
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
This patch set introduces driver XDP for veth.
Basically this is used in conjunction with redirect action of another XDP
program.
NIC -----------> veth===veth
(XDP) (redirect) (XDP)
In this case xdp_frame can be forwarded to the peer veth without
modification, so we can expect far better performance than generic XDP.
Envisioned use-cases
--------------------
* Container managed XDP program
Container host redirects frames to containers by XDP redirect action, and
privileged containers can deploy their own XDP programs.
* XDP program cascading
Two or more XDP programs can be called for each packet by redirecting
xdp frames to veth.
* Internal interface for an XDP bridge
When using XDP redirection to create a virtual bridge, veth can be used
to create an internal interface for the bridge.
Implementation
--------------
This changeset is making use of NAPI to implement ndo_xdp_xmit and
XDP_TX/REDIRECT. This is mainly because XDP heavily relies on NAPI
context.
- patch 1: Export a function needed for veth XDP.
- patch 2-3: Basic implementation of veth XDP.
- patch 4-5: Add ndo_xdp_xmit.
- patch 6-8: Add XDP_TX and XDP_REDIRECT.
- patch 9: Performance optimization for multi-queue env.
Tests and performance numbers
-----------------------------
Tested with a simple XDP program which only redirects packets between
NIC and veth. I used i40e 25G NIC (XXV710) for the physical NIC. The
server has 20 of Xeon Silver 2.20 GHz cores.
pktgen --(wire)--> XXV710 (i40e) <--(XDP redirect)--> veth===veth (XDP)
The rightmost veth loads XDP progs and just does DROP or TX. The number
of packets is measured in the XDP progs. The leftmost pktgen sends
packets at 37.1 Mpps (almost 25G wire speed).
veth XDP action Flows Mpps
================================
DROP 1 10.6
DROP 2 21.2
DROP 100 36.0
TX 1 5.0
TX 2 10.0
TX 100 31.0
I also measured netperf TCP_STREAM but was not so great performance due
to lack of tx/rx checksum offload and TSO, etc.
netperf <--(wire)--> XXV710 (i40e) <--(XDP redirect)--> veth===veth (XDP PASS)
Direction Flows Gbps
==============================
external->veth 1 20.8
external->veth 2 23.5
external->veth 100 23.6
veth->external 1 9.0
veth->external 2 17.8
veth->external 100 22.9
Also tested doing ifup/down or load/unload a XDP program repeatedly
during processing XDP packets in order to check if enabling/disabling
NAPI is working as expected, and found no problems.
v4:
- Don't adjust MTU automatically.
- Skip peer IFF_UP check on .ndo_xdp_xmit() because it is unnecessary.
Add comments to explain that.
- Use redirect_info instead of xdp_mem_info for storing no_direct flag
to avoid per packet copy cost.
v3:
- Drop skb bulk xmit patch since it makes little performance
difference. The hotspot in TCP skb xmit at this point is checksum
computation in skb_segment and packet copy on XDP_REDIRECT due to
cloned/nonlinear skb.
- Fix race on closing device.
- Add extack messages in ndo_bpf.
v2:
- Squash NAPI patch with "Add driver XDP" patch.
- Remove conversion from xdp_frame to skb when NAPI is not enabled.
- Introduce per-queue XDP ring (patch 8).
- Introduce bulk skb xmit when XDP is enabled on the peer (patch 9).
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Toshiaki Makita (9):
net: Export skb_headers_offset_update
veth: Add driver XDP
veth: Avoid drops by oversized packets when XDP is enabled
veth: Handle xdp_frames in xdp napi ring
veth: Add ndo_xdp_xmit
bpf: Make redirect_info accessible from modules
xdp: Helpers for disabling napi_direct of xdp_return_frame
veth: Add XDP TX and REDIRECT
veth: Support per queue XDP ring
drivers/net/veth.c | 747 ++++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/filter.h | 35 +++
include/linux/skbuff.h | 1 +
net/core/filter.c | 29 +-
net/core/skbuff.c | 3 +-
net/core/xdp.c | 6 +-
6 files changed, 791 insertions(+), 30 deletions(-)
--
2.14.3
^ permalink raw reply
* [PATCH v4 bpf-next 1/9] net: Export skb_headers_offset_update
From: Toshiaki Makita @ 2018-07-26 14:25 UTC (permalink / raw)
To: netdev, Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, Jesper Dangaard Brouer, Jakub Kicinski,
Toshiaki Makita
In-Reply-To: <20180726142557.1765-1-toshiaki.makita1@gmail.com>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
This is needed for veth XDP which does skb_copy_expand()-like operation.
v2:
- Drop skb_copy_header part because it has already been exported now.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index fd3cb1b247df..f6929688853a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1035,6 +1035,7 @@ static inline struct sk_buff *alloc_skb_fclone(unsigned int size,
}
struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src);
+void skb_headers_offset_update(struct sk_buff *skb, int off);
int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask);
struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t priority);
void skb_copy_header(struct sk_buff *new, const struct sk_buff *old);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 266b954f763e..f5670e6ab40c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1291,7 +1291,7 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
}
EXPORT_SYMBOL(skb_clone);
-static void skb_headers_offset_update(struct sk_buff *skb, int off)
+void skb_headers_offset_update(struct sk_buff *skb, int off)
{
/* Only adjust this if it actually is csum_start rather than csum */
if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -1305,6 +1305,7 @@ static void skb_headers_offset_update(struct sk_buff *skb, int off)
skb->inner_network_header += off;
skb->inner_mac_header += off;
}
+EXPORT_SYMBOL(skb_headers_offset_update);
void skb_copy_header(struct sk_buff *new, const struct sk_buff *old)
{
--
2.14.3
^ permalink raw reply related
* [PATCH v4 bpf-next 2/9] veth: Add driver XDP
From: Toshiaki Makita @ 2018-07-26 14:25 UTC (permalink / raw)
To: netdev, Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, Jesper Dangaard Brouer, Jakub Kicinski,
Toshiaki Makita
In-Reply-To: <20180726142557.1765-1-toshiaki.makita1@gmail.com>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
This is the basic implementation of veth driver XDP.
Incoming packets are sent from the peer veth device in the form of skb,
so this is generally doing the same thing as generic XDP.
This itself is not so useful, but a starting point to implement other
useful veth XDP features like TX and REDIRECT.
This introduces NAPI when XDP is enabled, because XDP is now heavily
relies on NAPI context. Use ptr_ring to emulate NIC ring. Tx function
enqueues packets to the ring and peer NAPI handler drains the ring.
Currently only one ring is allocated for each veth device, so it does
not scale on multiqueue env. This can be resolved by allocating rings
on the per-queue basis later.
Note that NAPI is not used but netif_rx is used when XDP is not loaded,
so this does not change the default behaviour.
v3:
- Fix race on closing the device.
- Add extack messages in ndo_bpf.
v2:
- Squashed with the patch adding NAPI.
- Implement adjust_tail.
- Don't acquire consumer lock because it is guarded by NAPI.
- Make poll_controller noop since it is unnecessary.
- Register rxq_info on enabling XDP rather than on opening the device.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
---
drivers/net/veth.c | 373 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 366 insertions(+), 7 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index a69ad39ee57e..78fa08cb6e24 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -19,10 +19,18 @@
#include <net/xfrm.h>
#include <linux/veth.h>
#include <linux/module.h>
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include <linux/ptr_ring.h>
+#include <linux/skb_array.h>
+#include <linux/bpf_trace.h>
#define DRV_NAME "veth"
#define DRV_VERSION "1.0"
+#define VETH_RING_SIZE 256
+#define VETH_XDP_HEADROOM (XDP_PACKET_HEADROOM + NET_IP_ALIGN)
+
struct pcpu_vstats {
u64 packets;
u64 bytes;
@@ -30,9 +38,16 @@ struct pcpu_vstats {
};
struct veth_priv {
+ struct napi_struct xdp_napi;
+ struct net_device *dev;
+ struct bpf_prog __rcu *xdp_prog;
+ struct bpf_prog *_xdp_prog;
struct net_device __rcu *peer;
atomic64_t dropped;
unsigned requested_headroom;
+ bool rx_notify_masked;
+ struct ptr_ring xdp_ring;
+ struct xdp_rxq_info xdp_rxq;
};
/*
@@ -98,11 +113,43 @@ static const struct ethtool_ops veth_ethtool_ops = {
.get_link_ksettings = veth_get_link_ksettings,
};
-static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
+/* general routines */
+
+static void __veth_xdp_flush(struct veth_priv *priv)
+{
+ /* Write ptr_ring before reading rx_notify_masked */
+ smp_mb();
+ if (!priv->rx_notify_masked) {
+ priv->rx_notify_masked = true;
+ napi_schedule(&priv->xdp_napi);
+ }
+}
+
+static int veth_xdp_rx(struct veth_priv *priv, struct sk_buff *skb)
+{
+ if (unlikely(ptr_ring_produce(&priv->xdp_ring, skb))) {
+ dev_kfree_skb_any(skb);
+ return NET_RX_DROP;
+ }
+
+ return NET_RX_SUCCESS;
+}
+
+static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb, bool xdp)
{
struct veth_priv *priv = netdev_priv(dev);
+
+ return __dev_forward_skb(dev, skb) ?: xdp ?
+ veth_xdp_rx(priv, skb) :
+ netif_rx(skb);
+}
+
+static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
struct net_device *rcv;
int length = skb->len;
+ bool rcv_xdp = false;
rcu_read_lock();
rcv = rcu_dereference(priv->peer);
@@ -111,7 +158,10 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
goto drop;
}
- if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
+ rcv_priv = netdev_priv(rcv);
+ rcv_xdp = rcu_access_pointer(rcv_priv->xdp_prog);
+
+ if (likely(veth_forward_skb(rcv, skb, rcv_xdp) == NET_RX_SUCCESS)) {
struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
u64_stats_update_begin(&stats->syncp);
@@ -122,14 +172,15 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
drop:
atomic64_inc(&priv->dropped);
}
+
+ if (rcv_xdp)
+ __veth_xdp_flush(rcv_priv);
+
rcu_read_unlock();
+
return NETDEV_TX_OK;
}
-/*
- * general routines
- */
-
static u64 veth_stats_one(struct pcpu_vstats *result, struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
@@ -179,18 +230,253 @@ static void veth_set_multicast_list(struct net_device *dev)
{
}
+static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
+ int buflen)
+{
+ struct sk_buff *skb;
+
+ if (!buflen) {
+ buflen = SKB_DATA_ALIGN(headroom + len) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ }
+ skb = build_skb(head, buflen);
+ if (!skb)
+ return NULL;
+
+ skb_reserve(skb, headroom);
+ skb_put(skb, len);
+
+ return skb;
+}
+
+static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
+ struct sk_buff *skb)
+{
+ u32 pktlen, headroom, act, metalen;
+ void *orig_data, *orig_data_end;
+ int size, mac_len, delta, off;
+ struct bpf_prog *xdp_prog;
+ struct xdp_buff xdp;
+
+ rcu_read_lock();
+ xdp_prog = rcu_dereference(priv->xdp_prog);
+ if (unlikely(!xdp_prog)) {
+ rcu_read_unlock();
+ goto out;
+ }
+
+ mac_len = skb->data - skb_mac_header(skb);
+ pktlen = skb->len + mac_len;
+ size = SKB_DATA_ALIGN(VETH_XDP_HEADROOM + pktlen) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ if (size > PAGE_SIZE)
+ goto drop;
+
+ headroom = skb_headroom(skb) - mac_len;
+ if (skb_shared(skb) || skb_head_is_locked(skb) ||
+ skb_is_nonlinear(skb) || headroom < XDP_PACKET_HEADROOM) {
+ struct sk_buff *nskb;
+ void *head, *start;
+ struct page *page;
+ int head_off;
+
+ page = alloc_page(GFP_ATOMIC);
+ if (!page)
+ goto drop;
+
+ head = page_address(page);
+ start = head + VETH_XDP_HEADROOM;
+ if (skb_copy_bits(skb, -mac_len, start, pktlen)) {
+ page_frag_free(head);
+ goto drop;
+ }
+
+ nskb = veth_build_skb(head,
+ VETH_XDP_HEADROOM + mac_len, skb->len,
+ PAGE_SIZE);
+ if (!nskb) {
+ page_frag_free(head);
+ goto drop;
+ }
+
+ skb_copy_header(nskb, skb);
+ head_off = skb_headroom(nskb) - skb_headroom(skb);
+ skb_headers_offset_update(nskb, head_off);
+ if (skb->sk)
+ skb_set_owner_w(nskb, skb->sk);
+ consume_skb(skb);
+ skb = nskb;
+ }
+
+ xdp.data_hard_start = skb->head;
+ xdp.data = skb_mac_header(skb);
+ xdp.data_end = xdp.data + pktlen;
+ xdp.data_meta = xdp.data;
+ xdp.rxq = &priv->xdp_rxq;
+ orig_data = xdp.data;
+ orig_data_end = xdp.data_end;
+
+ act = bpf_prog_run_xdp(xdp_prog, &xdp);
+
+ switch (act) {
+ case XDP_PASS:
+ break;
+ default:
+ bpf_warn_invalid_xdp_action(act);
+ case XDP_ABORTED:
+ trace_xdp_exception(priv->dev, xdp_prog, act);
+ case XDP_DROP:
+ goto drop;
+ }
+ rcu_read_unlock();
+
+ delta = orig_data - xdp.data;
+ off = mac_len + delta;
+ if (off > 0)
+ __skb_push(skb, off);
+ else if (off < 0)
+ __skb_pull(skb, -off);
+ skb->mac_header -= delta;
+ off = xdp.data_end - orig_data_end;
+ if (off != 0)
+ __skb_put(skb, off);
+ skb->protocol = eth_type_trans(skb, priv->dev);
+
+ metalen = xdp.data - xdp.data_meta;
+ if (metalen)
+ skb_metadata_set(skb, metalen);
+out:
+ return skb;
+drop:
+ rcu_read_unlock();
+ kfree_skb(skb);
+ return NULL;
+}
+
+static int veth_xdp_rcv(struct veth_priv *priv, int budget)
+{
+ int i, done = 0;
+
+ for (i = 0; i < budget; i++) {
+ struct sk_buff *skb = __ptr_ring_consume(&priv->xdp_ring);
+
+ if (!skb)
+ break;
+
+ skb = veth_xdp_rcv_skb(priv, skb);
+
+ if (skb)
+ napi_gro_receive(&priv->xdp_napi, skb);
+
+ done++;
+ }
+
+ return done;
+}
+
+static int veth_poll(struct napi_struct *napi, int budget)
+{
+ struct veth_priv *priv =
+ container_of(napi, struct veth_priv, xdp_napi);
+ int done;
+
+ done = veth_xdp_rcv(priv, budget);
+
+ if (done < budget && napi_complete_done(napi, done)) {
+ /* Write rx_notify_masked before reading ptr_ring */
+ smp_store_mb(priv->rx_notify_masked, false);
+ if (unlikely(!__ptr_ring_empty(&priv->xdp_ring))) {
+ priv->rx_notify_masked = true;
+ napi_schedule(&priv->xdp_napi);
+ }
+ }
+
+ return done;
+}
+
+static int veth_napi_add(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ int err;
+
+ err = ptr_ring_init(&priv->xdp_ring, VETH_RING_SIZE, GFP_KERNEL);
+ if (err)
+ return err;
+
+ netif_napi_add(dev, &priv->xdp_napi, veth_poll, NAPI_POLL_WEIGHT);
+ napi_enable(&priv->xdp_napi);
+
+ return 0;
+}
+
+static void veth_napi_del(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+
+ napi_disable(&priv->xdp_napi);
+ netif_napi_del(&priv->xdp_napi);
+ priv->rx_notify_masked = false;
+ ptr_ring_cleanup(&priv->xdp_ring, __skb_array_destroy_skb);
+}
+
+static int veth_enable_xdp(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ int err;
+
+ if (!xdp_rxq_info_is_reg(&priv->xdp_rxq)) {
+ err = xdp_rxq_info_reg(&priv->xdp_rxq, dev, 0);
+ if (err < 0)
+ return err;
+
+ err = xdp_rxq_info_reg_mem_model(&priv->xdp_rxq,
+ MEM_TYPE_PAGE_SHARED, NULL);
+ if (err < 0)
+ goto err;
+
+ err = veth_napi_add(dev);
+ if (err)
+ goto err;
+ }
+
+ rcu_assign_pointer(priv->xdp_prog, priv->_xdp_prog);
+
+ return 0;
+err:
+ xdp_rxq_info_unreg(&priv->xdp_rxq);
+
+ return err;
+}
+
+static void veth_disable_xdp(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+
+ rcu_assign_pointer(priv->xdp_prog, NULL);
+ veth_napi_del(dev);
+ xdp_rxq_info_unreg(&priv->xdp_rxq);
+}
+
static int veth_open(struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
struct net_device *peer = rtnl_dereference(priv->peer);
+ int err;
if (!peer)
return -ENOTCONN;
+ if (priv->_xdp_prog) {
+ err = veth_enable_xdp(dev);
+ if (err)
+ return err;
+ }
+
if (peer->flags & IFF_UP) {
netif_carrier_on(dev);
netif_carrier_on(peer);
}
+
return 0;
}
@@ -203,6 +489,9 @@ static int veth_close(struct net_device *dev)
if (peer)
netif_carrier_off(peer);
+ if (priv->_xdp_prog)
+ veth_disable_xdp(dev);
+
return 0;
}
@@ -228,7 +517,7 @@ static void veth_dev_free(struct net_device *dev)
static void veth_poll_controller(struct net_device *dev)
{
/* veth only receives frames when its peer sends one
- * Since it's a synchronous operation, we are guaranteed
+ * Since it has nothing to do with disabling irqs, we are guaranteed
* never to have pending data when we poll for it so
* there is nothing to do here.
*
@@ -276,6 +565,72 @@ static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
rcu_read_unlock();
}
+static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ struct bpf_prog *old_prog;
+ struct net_device *peer;
+ int err;
+
+ old_prog = priv->_xdp_prog;
+ priv->_xdp_prog = prog;
+ peer = rtnl_dereference(priv->peer);
+
+ if (prog) {
+ if (!peer) {
+ NL_SET_ERR_MSG_MOD(extack, "Cannot set XDP when peer is detached");
+ err = -ENOTCONN;
+ goto err;
+ }
+
+ if (dev->flags & IFF_UP) {
+ err = veth_enable_xdp(dev);
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack, "Setup for XDP failed");
+ goto err;
+ }
+ }
+ }
+
+ if (old_prog) {
+ if (!prog && dev->flags & IFF_UP)
+ veth_disable_xdp(dev);
+ bpf_prog_put(old_prog);
+ }
+
+ return 0;
+err:
+ priv->_xdp_prog = old_prog;
+
+ return err;
+}
+
+static u32 veth_xdp_query(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ const struct bpf_prog *xdp_prog;
+
+ xdp_prog = priv->_xdp_prog;
+ if (xdp_prog)
+ return xdp_prog->aux->id;
+
+ return 0;
+}
+
+static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
+{
+ switch (xdp->command) {
+ case XDP_SETUP_PROG:
+ return veth_xdp_set(dev, xdp->prog, xdp->extack);
+ case XDP_QUERY_PROG:
+ xdp->prog_id = veth_xdp_query(dev);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
static const struct net_device_ops veth_netdev_ops = {
.ndo_init = veth_dev_init,
.ndo_open = veth_open,
@@ -290,6 +645,7 @@ static const struct net_device_ops veth_netdev_ops = {
.ndo_get_iflink = veth_get_iflink,
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = veth_set_rx_headroom,
+ .ndo_bpf = veth_xdp,
};
#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
@@ -451,10 +807,13 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
*/
priv = netdev_priv(dev);
+ priv->dev = dev;
rcu_assign_pointer(priv->peer, peer);
priv = netdev_priv(peer);
+ priv->dev = peer;
rcu_assign_pointer(priv->peer, dev);
+
return 0;
err_register_dev:
--
2.14.3
^ permalink raw reply related
* [PATCH v4 bpf-next 3/9] veth: Avoid drops by oversized packets when XDP is enabled
From: Toshiaki Makita @ 2018-07-26 14:25 UTC (permalink / raw)
To: netdev, Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, Jesper Dangaard Brouer, Jakub Kicinski,
Toshiaki Makita
In-Reply-To: <20180726142557.1765-1-toshiaki.makita1@gmail.com>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
All oversized packets including GSO packets are dropped if XDP is
enabled on receiver side, so don't send such packets from peer.
Drop TSO and SCTP fragmentation features so that veth devices themselves
segment packets with XDP enabled. Also cap MTU accordingly.
v4:
- Don't auto-adjust MTU but cap max MTU.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
---
drivers/net/veth.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 78fa08cb6e24..1b4006d3df32 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -542,6 +542,23 @@ static int veth_get_iflink(const struct net_device *dev)
return iflink;
}
+static netdev_features_t veth_fix_features(struct net_device *dev,
+ netdev_features_t features)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ struct net_device *peer;
+
+ peer = rtnl_dereference(priv->peer);
+ if (peer) {
+ struct veth_priv *peer_priv = netdev_priv(peer);
+
+ if (peer_priv->_xdp_prog)
+ features &= ~NETIF_F_GSO_SOFTWARE;
+ }
+
+ return features;
+}
+
static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
{
struct veth_priv *peer_priv, *priv = netdev_priv(dev);
@@ -571,6 +588,7 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
struct veth_priv *priv = netdev_priv(dev);
struct bpf_prog *old_prog;
struct net_device *peer;
+ unsigned int max_mtu;
int err;
old_prog = priv->_xdp_prog;
@@ -584,6 +602,15 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
goto err;
}
+ max_mtu = PAGE_SIZE - VETH_XDP_HEADROOM -
+ peer->hard_header_len -
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ if (peer->mtu > max_mtu) {
+ NL_SET_ERR_MSG_MOD(extack, "Peer MTU is too large to set XDP");
+ err = -ERANGE;
+ goto err;
+ }
+
if (dev->flags & IFF_UP) {
err = veth_enable_xdp(dev);
if (err) {
@@ -591,14 +618,29 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
goto err;
}
}
+
+ if (!old_prog) {
+ peer->hw_features &= ~NETIF_F_GSO_SOFTWARE;
+ peer->max_mtu = max_mtu;
+ }
}
if (old_prog) {
- if (!prog && dev->flags & IFF_UP)
- veth_disable_xdp(dev);
+ if (!prog) {
+ if (dev->flags & IFF_UP)
+ veth_disable_xdp(dev);
+
+ if (peer) {
+ peer->hw_features |= NETIF_F_GSO_SOFTWARE;
+ peer->max_mtu = ETH_MAX_MTU;
+ }
+ }
bpf_prog_put(old_prog);
}
+ if ((!!old_prog ^ !!prog) && peer)
+ netdev_update_features(peer);
+
return 0;
err:
priv->_xdp_prog = old_prog;
@@ -643,6 +685,7 @@ static const struct net_device_ops veth_netdev_ops = {
.ndo_poll_controller = veth_poll_controller,
#endif
.ndo_get_iflink = veth_get_iflink,
+ .ndo_fix_features = veth_fix_features,
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = veth_set_rx_headroom,
.ndo_bpf = veth_xdp,
--
2.14.3
^ 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