Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
From: Erik Kline @ 2016-09-26  9:13 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev,
	Lorenzo Colitti
In-Reply-To: <1474801390-6891-2-git-send-email-zenczykowski@gmail.com>

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 2f1f5d439788..11fa1a5564d4 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -6044,8 +6044,14 @@ static int __addrconf_sysctl_register(struct net *net, char *dev_name,
>
>         for (i = 0; table[i].data; i++) {
>                 table[i].data += (char *)p - (char *)&ipv6_devconf;
> -               table[i].extra1 = idev; /* embedded; no ref */
> -               table[i].extra2 = net;
> +               /* If one of these is already set, then it is not safe to
> +                * overwrite either of them: this makes proc_dointvec_minmax
> +                * usable.
> +                */
> +               if (!table[i].extra1 && !table[i].extra2) {
> +                       table[i].extra1 = idev; /* embedded; no ref */
> +                       table[i].extra2 = net;
> +               }
>         }
>
>         snprintf(path, sizeof(path), "net/ipv6/conf/%s", dev_name);
> --
> 2.8.0.rc3.226.g39d4020
>

Acked-by: Erik Kline <ek@google.com>

^ permalink raw reply

* [PATCH v2] net: hns: mark symbols static where possible
From: Baoyou Xie @ 2016-09-26  9:13 UTC (permalink / raw)
  To: yisen.zhuang, davem, yankejian, huangdaode, lisheng011, lipeng321,
	xieqianqian, fabf, colin.king, geliangtang, arnd, andrew,
	chenny.xu, xypron.glpk
  Cc: netdev, linux-kernel, baoyou.xie, xie.baoyou, han.fei,
	tang.qiang007

We get a few warnings when building kernel with W=1:
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:76:21: warning: no previous prototype for 'hns_ae_get_handle' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:274:6: warning: no previous prototype for 'hns_ae_stop' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:302:6: warning: no previous prototype for 'hns_ae_toggle_ring_irq' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:490:6: warning: no previous prototype for 'hns_ae_update_stats' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:573:6: warning: no previous prototype for 'hns_ae_get_stats' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:605:6: warning: no previous prototype for 'hns_ae_get_strings' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:638:5: warning: no previous prototype for 'hns_ae_get_sset_count' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:687:6: warning: no previous prototype for 'hns_ae_update_led_status' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:698:5: warning: no previous prototype for 'hns_ae_cpld_set_led_id' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:710:6: warning: no previous prototype for 'hns_ae_get_regs' [-Wmissing-prototypes]
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:735:5: warning: no previous prototype for 'hns_ae_get_regs_len' [-Wmissing-prototypes]
....

In fact, these functions are only used in the file in which they are
declared and don't need a declaration, but can be made static.
so this patch marks these functions with 'static'.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/net/ethernet/hisilicon/hip04_eth.c         |  2 +-
 drivers/net/ethernet/hisilicon/hisi_femac.c        |  6 ++---
 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 |  7 ++---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c |  2 +-
 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      | 13 +++++-----
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c   | 24 +++++++++--------
 12 files changed, 57 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index 0c4afe9..6358dac 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -600,7 +600,7 @@ static irqreturn_t hip04_mac_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-enum hrtimer_restart tx_done(struct hrtimer *hrtimer)
+static enum hrtimer_restart tx_done(struct hrtimer *hrtimer)
 {
 	struct hip04_priv *priv;
 
diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
index b5d7ad0..65d5852 100644
--- a/drivers/net/ethernet/hisilicon/hisi_femac.c
+++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
@@ -940,8 +940,8 @@ static int hisi_femac_drv_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-int hisi_femac_drv_suspend(struct platform_device *pdev,
-			   pm_message_t state)
+static int hisi_femac_drv_suspend(struct platform_device *pdev,
+				  pm_message_t state)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct hisi_femac_priv *priv = netdev_priv(ndev);
@@ -957,7 +957,7 @@ int hisi_femac_drv_suspend(struct platform_device *pdev,
 	return 0;
 }
 
-int hisi_femac_drv_resume(struct platform_device *pdev)
+static int hisi_femac_drv_resume(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct hisi_femac_priv *priv = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
index e28d960..a1150e9 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
@@ -73,8 +73,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;
@@ -271,7 +271,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);
 
@@ -299,7 +299,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;
 
@@ -487,8 +487,8 @@ static void hns_ae_get_coalesce_range(struct hnae_handle *handle,
 	*rx_usecs_high  = HNS_RCB_MAX_COALESCED_USECS;
 }
 
-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;
@@ -570,7 +570,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;
@@ -602,8 +602,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;
@@ -635,7 +635,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;
@@ -684,7 +684,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;
 
@@ -695,8 +695,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;
 
@@ -707,7 +707,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;
@@ -732,7 +732,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 1e1eb92..f0c63c8 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
@@ -327,7 +327,7 @@ static void hns_gmac_init(void *mac_drv)
 		hns_gmac_set_uc_match(mac_drv, 0);
 }
 
-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 5c8afe1..43f957e 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -898,8 +898,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;
@@ -917,7 +918,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 afb5daa..36e27c7 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -41,7 +41,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;
@@ -2092,8 +2092,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;
 
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
index 611b67b..8ebe463 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
@@ -417,7 +417,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)
 {
 	if (!mac_cb->cpld_ctrl)
 		return -ENODEV;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
index 6ea8722..04e674c 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 ef11077..ded4506 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
@@ -612,7 +612,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:
@@ -648,7 +648,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 8f4f0e8..3198890 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
@@ -306,7 +306,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 d7e1f8c..1c72c66 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1085,7 +1085,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;
@@ -1373,7 +1373,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;
@@ -1486,7 +1486,7 @@ static netdev_features_t hns_nic_fix_features(
  *
  * 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;
@@ -1504,7 +1504,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;
@@ -1519,8 +1519,9 @@ void hns_nic_set_rx_mode(struct net_device *ndev)
 	hns_set_multicast_list(ndev);
 }
 
-struct rtnl_link_stats64 *hns_nic_get_stats64(struct net_device *ndev,
-					      struct rtnl_link_stats64 *stats)
+static struct rtnl_link_stats64 *
+hns_nic_get_stats64(struct net_device *ndev,
+		    struct rtnl_link_stats64 *stats)
 {
 	int idx = 0;
 	u64 tx_bytes = 0;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index ab33487..e538077 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -666,8 +666,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;
@@ -815,7 +815,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);
 
@@ -832,8 +833,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);
@@ -890,7 +891,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;
@@ -980,7 +981,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;
@@ -1012,7 +1013,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 hns_nic_priv *priv = netdev_priv(netdev);
@@ -1035,7 +1036,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;
@@ -1109,8 +1111,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.4

^ permalink raw reply related

* Re: [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
From: Erik Kline @ 2016-09-26  9:14 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev,
	Lorenzo Colitti
In-Reply-To: <1474801390-6891-3-git-send-email-zenczykowski@gmail.com>

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> replace with extra1/2 magic
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 11fa1a5564d4..3a835495fb53 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5467,20 +5467,6 @@ int addrconf_sysctl_forward(struct ctl_table *ctl, int write,
>  }
>
>  static
> -int addrconf_sysctl_hop_limit(struct ctl_table *ctl, int write,
> -                              void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -       struct ctl_table lctl;
> -       int min_hl = 1, max_hl = 255;
> -
> -       lctl = *ctl;
> -       lctl.extra1 = &min_hl;
> -       lctl.extra2 = &max_hl;
> -
> -       return proc_dointvec_minmax(&lctl, write, buffer, lenp, ppos);
> -}
> -
> -static
>  int addrconf_sysctl_mtu(struct ctl_table *ctl, int write,
>                         void __user *buffer, size_t *lenp, loff_t *ppos)
>  {
> @@ -5713,6 +5699,9 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
>         return ret;
>  }
>
> +static int one = 1;
> +static int two_five_five = 255;

Should these be const as well?

> +
>  static const struct ctl_table addrconf_sysctl[] = {
>         {
>                 .procname       = "forwarding",
> @@ -5726,7 +5715,9 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .data           = &ipv6_devconf.hop_limit,
>                 .maxlen         = sizeof(int),
>                 .mode           = 0644,
> -               .proc_handler   = addrconf_sysctl_hop_limit,
> +               .proc_handler   = proc_dointvec_minmax,
> +               .extra1         = &one,
> +               .extra2         = &two_five_five,
>         },
>         {
>                 .procname       = "mtu",
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply

* Re: [PATCH 5/5] ISDN-CAPI: Delete unnecessary braces
From: Paul Bolle @ 2016-09-26  9:20 UTC (permalink / raw)
  To: SF Markus Elfring, Sergei Shtylyov
  Cc: netdev, Karsten Keil, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <727b14d4-7a3b-9de8-25d9-c3bf212f7c44@users.sourceforge.net>

On Sun, 2016-09-25 at 14:47 +0200, SF Markus Elfring wrote:
> > > @@ -976,13 +974,12 @@ static void handle_controller(_cmsg *cmsg)
> > >          if (debugmode)
> > >              printk(KERN_DEBUG "capidrv-%d: listenconf Info=0x%4x
> > > (%s) cipmask=0x%x\n",
> > >                     card->contrnr, cmsg->Info,
> > > capi_info2str(cmsg->Info), card->cipmask);
> > > -        if (cmsg->Info) {
> > > +        if (cmsg->Info)
> > >              listen_change_state(card, EV_LISTEN_CONF_ERROR);
> > > -        } else if (card->cipmask == 0) {
> > > +        else if (card->cipmask == 0)
> > >              listen_change_state(card, EV_LISTEN_CONF_EMPTY);
> > > -        } else {
> > > +             else
> > 
> >    Indented too much.
> 
> How do you think about an alignment of this "else"
> with the corresponding if statement three lines above?

Well, I think it looks silly. checkpatch apparently agrees:
    WARNING: Statements should start on a tabstop
    #51: FILE: drivers/isdn/capi/capidrv.c:981:
    +		     else
    
    total: 0 errors, 1 warnings, 91 lines checked
    
    NOTE: For some of the reported defects, checkpatch may be able to
          mechanically convert to the typical style using --fix or --fix-inplace.
    
    Your patch has style problems, please review.
    
    NOTE: If any of the errors are false positives, please report
          them to the maintainer, see CHECKPATCH in MAINTAINERS.

You use checkpatch a lot, don't you? Didn't you use it to, you know,
check your patch?


Paul Bolle

^ permalink raw reply

* Re: [PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
From: Erik Kline @ 2016-09-26  9:23 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev,
	Lorenzo Colitti
In-Reply-To: <1474801390-6891-4-git-send-email-zenczykowski@gmail.com>

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
> to -1 meaning an unlimited number of retransmits.
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3a835495fb53..6c63bf06fbcf 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
>         if (idev->if_flags & IF_RA_RCVD)
>                 goto out;
>
> -       if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
> +       if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
>                 write_unlock(&idev->lock);
>                 if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
>                         ndisc_send_rs(dev, &lladdr,
> @@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
>         send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
>         send_rs = send_mld &&
>                   ipv6_accept_ra(ifp->idev) &&
> -                 ifp->idev->cnf.rtr_solicits > 0 &&
> +                 ifp->idev->cnf.rtr_solicits != 0 &&
>                   (dev->flags&IFF_LOOPBACK) == 0;
>         read_unlock_bh(&ifp->idev->lock);
>
> @@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
>                 return -EINVAL;
>         if (!ipv6_accept_ra(idev))
>                 return -EINVAL;
> -       if (idev->cnf.rtr_solicits <= 0)
> +       if (idev->cnf.rtr_solicits == 0)
>                 return -EINVAL;
>
>         write_lock_bh(&idev->lock);
> @@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
>         return ret;
>  }
>
> +static int minus_one = -1;

Same question from part 2: const as well?

>  static int one = 1;
>  static int two_five_five = 255;
>
> @@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .data           = &ipv6_devconf.rtr_solicits,
>                 .maxlen         = sizeof(int),
>                 .mode           = 0644,
> -               .proc_handler   = proc_dointvec,
> +               .proc_handler   = proc_dointvec_minmax,
> +               .extra1         = &minus_one,
>         },
>         {
>                 .procname       = "router_solicitation_interval",
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply

* Re: [PATCH net-next] net/sched: pkt_cls: change tc actions order to be as the user sets
From: Jamal Hadi Salim @ 2016-09-26  9:29 UTC (permalink / raw)
  To: Hadar Hen Zion, Cong Wang
  Cc: Hadar Hen Zion, David S. Miller, Linux Kernel Network Developers,
	Or Gerlitz
In-Reply-To: <CAJL1qvGnPsjbzp9Z_6GkXzJqrxAAgYY6uE_0mCWb=H-EhcB8-A@mail.gmail.com>

On 16-09-26 02:02 AM, Hadar Hen Zion wrote:
> On Mon, Sep 26, 2016 at 7:31 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:

>>
>> The reason is we use action->order as an nested attribute, so
>> the order in the list doesn't matter, only action->order itself matters.
>
> The order in the list matters for offload drivers who use the
> "tcf_exts_to_list" function and action->order parameter isn't usable
> for them.

Ok, thanks for the explanation.

> Why not keeping the actions in the same order as the user? isn't it
> more elegant?
>

I dont think it was intentional. I will ACK the patch.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
From: Erik Kline @ 2016-09-26  9:29 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev,
	Lorenzo Colitti
In-Reply-To: <1474801390-6891-5-git-send-email-zenczykowski@gmail.com>

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> Accessible via:
>   /proc/sys/net/ipv6/conf/*/router_solicitation_max_interval
>
> For now we default it to the same value as the normal interval.
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  include/linux/ipv6.h      |  1 +
>  include/net/addrconf.h    |  1 +
>  include/uapi/linux/ipv6.h |  1 +
>  net/ipv6/addrconf.c       | 11 +++++++++++
>  4 files changed, 14 insertions(+)
>
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index c6dbcd84a2c7..7e9a789be5e0 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -18,6 +18,7 @@ struct ipv6_devconf {
>         __s32           dad_transmits;
>         __s32           rtr_solicits;
>         __s32           rtr_solicit_interval;
> +       __s32           rtr_solicit_max_interval;
>         __s32           rtr_solicit_delay;
>         __s32           force_mld_version;
>         __s32           mldv1_unsolicited_report_interval;
> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> index 9826d3a9464c..275e5af4c2f4 100644
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -3,6 +3,7 @@
>
>  #define MAX_RTR_SOLICITATIONS          3
>  #define RTR_SOLICITATION_INTERVAL      (4*HZ)
> +#define RTR_SOLICITATION_MAX_INTERVAL  (4*HZ)
>
>  #define MIN_VALID_LIFETIME             (2*3600)        /* 2 hours */
>
> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> index 395876060f50..8c2772340c3f 100644
> --- a/include/uapi/linux/ipv6.h
> +++ b/include/uapi/linux/ipv6.h
> @@ -177,6 +177,7 @@ enum {
>         DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
>         DEVCONF_DROP_UNSOLICITED_NA,
>         DEVCONF_KEEP_ADDR_ON_DOWN,
> +       DEVCONF_RTR_SOLICIT_MAX_INTERVAL,
>         DEVCONF_MAX
>  };
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 6c63bf06fbcf..255be34cdbce 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -187,6 +187,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
>         .dad_transmits          = 1,
>         .rtr_solicits           = MAX_RTR_SOLICITATIONS,
>         .rtr_solicit_interval   = RTR_SOLICITATION_INTERVAL,
> +       .rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
>         .rtr_solicit_delay      = MAX_RTR_SOLICITATION_DELAY,
>         .use_tempaddr           = 0,
>         .temp_valid_lft         = TEMP_VALID_LIFETIME,
> @@ -232,6 +233,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
>         .dad_transmits          = 1,
>         .rtr_solicits           = MAX_RTR_SOLICITATIONS,
>         .rtr_solicit_interval   = RTR_SOLICITATION_INTERVAL,
> +       .rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
>         .rtr_solicit_delay      = MAX_RTR_SOLICITATION_DELAY,
>         .use_tempaddr           = 0,
>         .temp_valid_lft         = TEMP_VALID_LIFETIME,
> @@ -4891,6 +4893,8 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
>         array[DEVCONF_RTR_SOLICITS] = cnf->rtr_solicits;
>         array[DEVCONF_RTR_SOLICIT_INTERVAL] =
>                 jiffies_to_msecs(cnf->rtr_solicit_interval);
> +       array[DEVCONF_RTR_SOLICIT_MAX_INTERVAL] =
> +               jiffies_to_msecs(cnf->rtr_solicit_max_interval);
>         array[DEVCONF_RTR_SOLICIT_DELAY] =
>                 jiffies_to_msecs(cnf->rtr_solicit_delay);
>         array[DEVCONF_FORCE_MLD_VERSION] = cnf->force_mld_version;
> @@ -5771,6 +5775,13 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .proc_handler   = proc_dointvec_jiffies,
>         },
>         {
> +               .procname       = "router_solicitation_max_interval",
> +               .data           = &ipv6_devconf.rtr_solicit_max_interval,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0644,
> +               .proc_handler   = proc_dointvec_jiffies,
> +       },
> +       {
>                 .procname       = "router_solicitation_delay",
>                 .data           = &ipv6_devconf.rtr_solicit_delay,
>                 .maxlen         = sizeof(int),
> --
> 2.8.0.rc3.226.g39d4020
>

Acked-by: Erik Kline <ek@google.com>

^ permalink raw reply

* RE: [PATCH 3/3] net: fec: align IP header in hardware
From: David Laight @ 2016-09-26  9:26 UTC (permalink / raw)
  To: 'Eric Nelson', netdev@vger.kernel.org
  Cc: linux@arm.linux.org.uk, andrew@lunn.ch, fugang.duan@nxp.com,
	otavio@ossystems.com.br, edumazet@google.com,
	troy.kisky@boundarydevices.com, davem@davemloft.net,
	u.kleine-koenig@pengutronix.de
In-Reply-To: <1474728139-9335-4-git-send-email-eric@nelint.com>

From: Eric Nelson
> Sent: 24 September 2016 15:42
> The FEC receive accelerator (RACC) supports shifting the data payload of
> received packets by 16-bits, which aligns the payload (IP header) on a
> 4-byte boundary, which is, if not required, at least strongly suggested
> by the Linux networking layer.
...
> +		/* align IP header */
> +		val |= FEC_RACC_SHIFT16;

I can't help feeling that there needs to be corresponding
changes to increase the buffer size by 2 (maybe for large mtu)
and to discard two bytes from the frame length.

If probably ought to be predicated on NET_IP_ALIGN as well.

	David

^ permalink raw reply

* RE: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: David Laight @ 2016-09-26  9:33 UTC (permalink / raw)
  To: 'Joe Perches', Jean Delvare
  Cc: Larry Finger, Chaoming Li, Kalle Valo,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1474733754.23838.3.camel@perches.com>

> If you want to create enum->#ENUM structs and
> "const char *" lookup functions, please be my guest.
> 
> otherwise, hex is at least a consistent way to display
> what should be infrequent output.

If I've typed it right:

#define tags(x) x(A) x(B) x(C)
#define x(t) t,
enum {tags(x) tag_count};
#undef x
#define x(t) ##t,
static const char names[] = { tags(x) };
#undef x

	David

^ permalink raw reply

* Re: [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
From: Maciej Żenczykowski @ 2016-09-26  9:37 UTC (permalink / raw)
  To: Erik Kline; +Cc: David S . Miller, netdev, Lorenzo Colitti
In-Reply-To: <CAAedzxoD6DiNDQUpuiV3tQV0zVmAb=J6968aU02Z+3521u0s3Q@mail.gmail.com>

>> +static int one = 1;
>> +static int two_five_five = 255;
>
> Should these be const as well?

It would be nice, but you actually get compile time warnings if you do that.

^ permalink raw reply

* [PATCH net 0/5] sctp: some fixes of prsctp polices
From: Xin Long @ 2016-09-26  9:47 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel

This patchset is to improve some codes about prsctp polices, and also
to fix some issues.

Xin Long (5):
  sctp: move sent_count to the memory hole in sctp_chunk
  sctp: reuse sent_count to avoid retransmitted chunks for RTT
    measurements
  sctp: remove prsctp_param from sctp_chunk
  sctp: change to check peer prsctp_capable when using prsctp polices
  sctp: remove the old ttl expires policy

 include/net/sctp/structs.h | 15 +++------------
 net/sctp/chunk.c           | 31 ++++++++++---------------------
 net/sctp/output.c          |  6 ++----
 net/sctp/outqueue.c        | 16 +++++++---------
 net/sctp/sm_make_chunk.c   | 15 ---------------
 5 files changed, 22 insertions(+), 61 deletions(-)

-- 
2.1.0

^ permalink raw reply

* [PATCH net 1/5] sctp: move sent_count to the memory hole in sctp_chunk
From: Xin Long @ 2016-09-26  9:47 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1474883200.git.lucien.xin@gmail.com>

Now pahole sctp_chunk, it has 2 memory holes:
   struct sctp_chunk {
	struct list_head           list;
	atomic_t                   refcnt;
	/* XXX 4 bytes hole, try to pack */
	...
	long unsigned int          prsctp_param;
	int                        sent_count;
	/* XXX 4 bytes hole, try to pack */

This patch is to move up sent_count to fill the 1st one and eliminate
the 2nd one.

It's not just another struct compaction, it also fixes the "netperf-
Throughput_Mbps -37.2% regression" issue when overloading the CPU.

Fixes: a6c2f792873a ("sctp: implement prsctp TTL policy")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index ce93c4b..4f097f5 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -554,6 +554,9 @@ struct sctp_chunk {
 
 	atomic_t refcnt;
 
+	/* How many times this chunk have been sent, for prsctp RTX policy */
+	int sent_count;
+
 	/* This is our link to the per-transport transmitted list.  */
 	struct list_head transmitted_list;
 
@@ -610,9 +613,6 @@ struct sctp_chunk {
 	 */
 	unsigned long prsctp_param;
 
-	/* How many times this chunk have been sent, for prsctp RTX policy */
-	int sent_count;
-
 	/* Which association does this belong to?  */
 	struct sctp_association *asoc;
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH net 2/5] sctp: reuse sent_count to avoid retransmitted chunks for RTT measurements
From: Xin Long @ 2016-09-26  9:47 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1474883200.git.lucien.xin@gmail.com>

Now sctp uses chunk->resent to record if a chunk is retransmitted, for
RTT measurements with retransmitted DATA chunks. chunk->sent_count was
introduced to record how many times one chunk has been sent for prsctp
RTX policy before. We actually can know if one chunk is retransmitted
by checking chunk->sent_count is greater than 1.

This patch is to remove resent from sctp_chunk and reuse sent_count
to avoid retransmitted chunks for RTT measurements.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 1 -
 net/sctp/output.c          | 3 ++-
 net/sctp/outqueue.c        | 4 +---
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 4f097f5..81a5faa 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -647,7 +647,6 @@ struct sctp_chunk {
 #define SCTP_NEED_FRTX 0x1
 #define SCTP_DONT_FRTX 0x2
 	__u16	rtt_in_progress:1,	/* This chunk used for RTT calc? */
-		resent:1,		/* Has this chunk ever been resent. */
 		has_tsn:1,		/* Does this chunk have a TSN yet? */
 		has_ssn:1,		/* Does this chunk have a SSN yet? */
 		singleton:1,		/* Only chunk in the packet? */
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 31b7bc3..483d2ed 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -547,7 +547,8 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
 				 * for a given destination transport address.
 				 */
 
-				if (!chunk->resent && !tp->rto_pending) {
+				if (chunk->sent_count == 1 &&
+				    !tp->rto_pending) {
 					chunk->rtt_in_progress = 1;
 					tp->rto_pending = 1;
 				}
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 72e54a4..2a629fa 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -536,8 +536,6 @@ void sctp_retransmit_mark(struct sctp_outq *q,
 				transport->rto_pending = 0;
 			}
 
-			chunk->resent = 1;
-
 			/* Move the chunk to the retransmit queue. The chunks
 			 * on the retransmit queue are always kept in order.
 			 */
@@ -1467,7 +1465,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
 				 * instance).
 				 */
 				if (!tchunk->tsn_gap_acked &&
-				    !tchunk->resent &&
+				    tchunk->sent_count == 1 &&
 				    tchunk->rtt_in_progress) {
 					tchunk->rtt_in_progress = 0;
 					rtt = jiffies - tchunk->sent_at;
-- 
2.1.0

^ permalink raw reply related

* [PATCH net 3/5] sctp: remove prsctp_param from sctp_chunk
From: Xin Long @ 2016-09-26  9:47 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1474883200.git.lucien.xin@gmail.com>

Now sctp uses chunk->prsctp_param to save the prsctp param for all the
prsctp polices, we didn't need to introduce prsctp_param to sctp_chunk.
We can just use chunk->sinfo.sinfo_timetolive for RTX and BUF polices,
and reuse msg->expires_at for TTL policy, as the prsctp polices and old
expires policy are mutual exclusive.

This patch is to remove prsctp_param from sctp_chunk, and reuse msg's
expires_at for TTL and chunk's sinfo.sinfo_timetolive for RTX and BUF
polices.

Note that sctp can't use chunk's sinfo.sinfo_timetolive for TTL policy,
as it needs a u64 variables to save the expires_at time.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h |  7 -------
 net/sctp/chunk.c           |  9 +++++++--
 net/sctp/outqueue.c        |  4 ++--
 net/sctp/sm_make_chunk.c   | 15 ---------------
 4 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 81a5faa..ed74ba0 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -606,13 +606,6 @@ struct sctp_chunk {
 	/* This needs to be recoverable for SCTP_SEND_FAILED events. */
 	struct sctp_sndrcvinfo sinfo;
 
-	/* We use this field to record param for prsctp policies,
-	 * for TTL policy, it is the time_to_drop of this chunk,
-	 * for RTX policy, it is the max_sent_count of this chunk,
-	 * for PRIO policy, it is the priority of this chunk.
-	 */
-	unsigned long prsctp_param;
-
 	/* Which association does this belong to?  */
 	struct sctp_association *asoc;
 
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index a55e547..14990e2 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -179,6 +179,11 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 			 msg, msg->expires_at, jiffies);
 	}
 
+	if (asoc->prsctp_enable &&
+	    SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
+		msg->expires_at =
+			jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
+
 	/* This is the biggest possible DATA chunk that can fit into
 	 * the packet
 	 */
@@ -349,14 +354,14 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 	}
 
 	if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
-	    time_after(jiffies, chunk->prsctp_param)) {
+	    time_after(jiffies, chunk->msg->expires_at)) {
 		if (chunk->sent_count)
 			chunk->asoc->abandoned_sent[SCTP_PR_INDEX(TTL)]++;
 		else
 			chunk->asoc->abandoned_unsent[SCTP_PR_INDEX(TTL)]++;
 		return 1;
 	} else if (SCTP_PR_RTX_ENABLED(chunk->sinfo.sinfo_flags) &&
-		   chunk->sent_count > chunk->prsctp_param) {
+		   chunk->sent_count > chunk->sinfo.sinfo_timetolive) {
 		chunk->asoc->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
 		return 1;
 	}
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 2a629fa..df6bb59 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -383,7 +383,7 @@ static int sctp_prsctp_prune_sent(struct sctp_association *asoc,
 
 	list_for_each_entry_safe(chk, temp, queue, transmitted_list) {
 		if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
-		    chk->prsctp_param <= sinfo->sinfo_timetolive)
+		    chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
 			continue;
 
 		list_del_init(&chk->transmitted_list);
@@ -418,7 +418,7 @@ static int sctp_prsctp_prune_unsent(struct sctp_association *asoc,
 
 	list_for_each_entry_safe(chk, temp, queue, list) {
 		if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
-		    chk->prsctp_param <= sinfo->sinfo_timetolive)
+		    chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
 			continue;
 
 		list_del_init(&chk->list);
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 8c77b87..46ffecc 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -706,20 +706,6 @@ nodata:
 	return retval;
 }
 
-static void sctp_set_prsctp_policy(struct sctp_chunk *chunk,
-				   const struct sctp_sndrcvinfo *sinfo)
-{
-	if (!chunk->asoc->prsctp_enable)
-		return;
-
-	if (SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
-		chunk->prsctp_param =
-			jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
-	else if (SCTP_PR_RTX_ENABLED(sinfo->sinfo_flags) ||
-		 SCTP_PR_PRIO_ENABLED(sinfo->sinfo_flags))
-		chunk->prsctp_param = sinfo->sinfo_timetolive;
-}
-
 /* Make a DATA chunk for the given association from the provided
  * parameters.  However, do not populate the data payload.
  */
@@ -753,7 +739,6 @@ struct sctp_chunk *sctp_make_datafrag_empty(struct sctp_association *asoc,
 
 	retval->subh.data_hdr = sctp_addto_chunk(retval, sizeof(dp), &dp);
 	memcpy(&retval->sinfo, sinfo, sizeof(struct sctp_sndrcvinfo));
-	sctp_set_prsctp_policy(retval, sinfo);
 
 nodata:
 	return retval;
-- 
2.1.0

^ permalink raw reply related

* [PATCH net 4/5] sctp: change to check peer prsctp_capable when using prsctp polices
From: Xin Long @ 2016-09-26  9:47 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1474883200.git.lucien.xin@gmail.com>

Now before using prsctp polices, sctp uses asoc->prsctp_enable to
check if prsctp is enabled. However asoc->prsctp_enable is set only
means local host support prsctp, sctp should not abandon packet if
peer host doesn't enable prsctp.

So this patch is to use asoc->peer.prsctp_capable to check if prsctp
is enabled on both side, instead of asoc->prsctp_enable, as asoc's
peer.prsctp_capable is set only when local and peer both enable prsctp.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/chunk.c    | 4 ++--
 net/sctp/outqueue.c | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 14990e2..0a3dbec 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -179,7 +179,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 			 msg, msg->expires_at, jiffies);
 	}
 
-	if (asoc->prsctp_enable &&
+	if (asoc->peer.prsctp_capable &&
 	    SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
 		msg->expires_at =
 			jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
@@ -340,7 +340,7 @@ errout:
 /* Check whether this message has expired. */
 int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 {
-	if (!chunk->asoc->prsctp_enable ||
+	if (!chunk->asoc->peer.prsctp_capable ||
 	    !SCTP_PR_POLICY(chunk->sinfo.sinfo_flags)) {
 		struct sctp_datamsg *msg = chunk->msg;
 
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index df6bb59..a549363 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -326,7 +326,7 @@ int sctp_outq_tail(struct sctp_outq *q, struct sctp_chunk *chunk, gfp_t gfp)
 
 			sctp_chunk_hold(chunk);
 			sctp_outq_tail_data(q, chunk);
-			if (chunk->asoc->prsctp_enable &&
+			if (chunk->asoc->peer.prsctp_capable &&
 			    SCTP_PR_PRIO_ENABLED(chunk->sinfo.sinfo_flags))
 				chunk->asoc->sent_cnt_removable++;
 			if (chunk->chunk_hdr->flags & SCTP_DATA_UNORDERED)
@@ -442,7 +442,7 @@ void sctp_prsctp_prune(struct sctp_association *asoc,
 {
 	struct sctp_transport *transport;
 
-	if (!asoc->prsctp_enable || !asoc->sent_cnt_removable)
+	if (!asoc->peer.prsctp_capable || !asoc->sent_cnt_removable)
 		return;
 
 	msg_len = sctp_prsctp_prune_sent(asoc, sinfo,
@@ -1053,7 +1053,7 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout, gfp_t gfp)
 
 				/* Mark as failed send. */
 				sctp_chunk_fail(chunk, SCTP_ERROR_INV_STRM);
-				if (asoc->prsctp_enable &&
+				if (asoc->peer.prsctp_capable &&
 				    SCTP_PR_PRIO_ENABLED(chunk->sinfo.sinfo_flags))
 					asoc->sent_cnt_removable--;
 				sctp_chunk_free(chunk);
@@ -1345,7 +1345,7 @@ int sctp_outq_sack(struct sctp_outq *q, struct sctp_chunk *chunk)
 		tsn = ntohl(tchunk->subh.data_hdr->tsn);
 		if (TSN_lte(tsn, ctsn)) {
 			list_del_init(&tchunk->transmitted_list);
-			if (asoc->prsctp_enable &&
+			if (asoc->peer.prsctp_capable &&
 			    SCTP_PR_PRIO_ENABLED(chunk->sinfo.sinfo_flags))
 				asoc->sent_cnt_removable--;
 			sctp_chunk_free(tchunk);
-- 
2.1.0

^ permalink raw reply related

* [PATCH net 5/5] sctp: remove the old ttl expires policy
From: Xin Long @ 2016-09-26  9:47 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1474883200.git.lucien.xin@gmail.com>

The prsctp polices include ttl expires policy already, we should remove
the old ttl expires codes, and just adjust the new polices' codes to be
compatible with the old one for users.

This patch is to remove all the old expires codes, and if prsctp polices
are not set, it will still set msg's expires_at and check the expires in
sctp_check_abandoned.

Note that asoc->prsctp_enable is set by default, so users can't feel any
difference even if they use the old expires api in userspace.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h |  1 -
 net/sctp/chunk.c           | 32 ++++++++------------------------
 net/sctp/output.c          |  3 ---
 3 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index ed74ba0..a4a18f1 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -530,7 +530,6 @@ struct sctp_datamsg {
 	/* Did the messenge fail to send? */
 	int send_error;
 	u8 send_failed:1,
-	   can_abandon:1,   /* can chunks from this message can be abandoned. */
 	   can_delay;	    /* should this message be Nagle delayed */
 };
 
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 0a3dbec..8d366fc 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -52,7 +52,6 @@ static void sctp_datamsg_init(struct sctp_datamsg *msg)
 	atomic_set(&msg->refcnt, 1);
 	msg->send_failed = 0;
 	msg->send_error = 0;
-	msg->can_abandon = 0;
 	msg->can_delay = 1;
 	msg->expires_at = 0;
 	INIT_LIST_HEAD(&msg->chunks);
@@ -169,20 +168,11 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 	/* Note: Calculate this outside of the loop, so that all fragments
 	 * have the same expiration.
 	 */
-	if (sinfo->sinfo_timetolive) {
-		/* sinfo_timetolive is in milliseconds */
+	if (asoc->peer.prsctp_capable && sinfo->sinfo_timetolive &&
+	    (SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags) ||
+	     !SCTP_PR_POLICY(sinfo->sinfo_flags)))
 		msg->expires_at = jiffies +
 				    msecs_to_jiffies(sinfo->sinfo_timetolive);
-		msg->can_abandon = 1;
-
-		pr_debug("%s: msg:%p expires_at:%ld jiffies:%ld\n", __func__,
-			 msg, msg->expires_at, jiffies);
-	}
-
-	if (asoc->peer.prsctp_capable &&
-	    SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
-		msg->expires_at =
-			jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
 
 	/* This is the biggest possible DATA chunk that can fit into
 	 * the packet
@@ -340,18 +330,8 @@ errout:
 /* Check whether this message has expired. */
 int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 {
-	if (!chunk->asoc->peer.prsctp_capable ||
-	    !SCTP_PR_POLICY(chunk->sinfo.sinfo_flags)) {
-		struct sctp_datamsg *msg = chunk->msg;
-
-		if (!msg->can_abandon)
-			return 0;
-
-		if (time_after(jiffies, msg->expires_at))
-			return 1;
-
+	if (!chunk->asoc->peer.prsctp_capable)
 		return 0;
-	}
 
 	if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
 	    time_after(jiffies, chunk->msg->expires_at)) {
@@ -364,6 +344,10 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 		   chunk->sent_count > chunk->sinfo.sinfo_timetolive) {
 		chunk->asoc->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
 		return 1;
+	} else if (!SCTP_PR_POLICY(chunk->sinfo.sinfo_flags) &&
+		   chunk->msg->expires_at &&
+		   time_after(jiffies, chunk->msg->expires_at)) {
+		return 1;
 	}
 	/* PRIO policy is processed by sendmsg, not here */
 
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 483d2ed..e50079d 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -868,9 +868,6 @@ static void sctp_packet_append_data(struct sctp_packet *packet,
 		rwnd = 0;
 
 	asoc->peer.rwnd = rwnd;
-	/* Has been accepted for transmission. */
-	if (!asoc->peer.prsctp_capable)
-		chunk->msg->can_abandon = 0;
 	sctp_chunk_assign_tsn(chunk);
 	sctp_chunk_assign_ssn(chunk);
 }
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH net-next] net/sched: pkt_cls: change tc actions order to be as the user sets
From: Jamal Hadi Salim @ 2016-09-26  9:52 UTC (permalink / raw)
  To: Hadar Hen Zion, David S. Miller; +Cc: netdev, Cong Wang, Or Gerlitz
In-Reply-To: <1474812524-29089-1-git-send-email-hadarh@mellanox.com>

On 16-09-25 10:08 AM, Hadar Hen Zion wrote:
> Currently the created tc actions list is reversed against the order
> set by the user.
> Change the actions list order to be the same as was set by the user.
>
> Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

^ permalink raw reply

* Re: [PATCH net v2 0/2] Fix tc-ife bugs
From: Jamal Hadi Salim @ 2016-09-26  9:55 UTC (permalink / raw)
  To: Yotam Gigi, Yotam Gigi, davem@davemloft.net,
	netdev@vger.kernel.org
  Cc: Jiri Pirko, Elad Raz, mlxsw, Roman Mashak
In-Reply-To: <DB3PR05MB07641B3E085814C60BC60D64ACCD0@DB3PR05MB0764.eurprd05.prod.outlook.com>

On 16-09-26 03:01 AM, Yotam Gigi wrote:
>
>
>> -----Original Message-----
>> From: Jamal Hadi Salim [mailto:jhs@mojatatu.com]
>> Sent: Monday, September 26, 2016 3:47 AM
>> To: Yotam Gigi <yotamg@mellanox.com>; Yotam Gigi <yotam.gi@gmail.com>;

[..]
>>> Yes please - Add NLA_HDRLEN to the dlen on the encode you showed above.
>
> OK. Did it and tested it. Everything seems functional.
>

Thank you. Please resend with this change and add my ACK to both
(I tested the first one)

>>>
>>
>> And the correct commit it fixes is:
>> a823f93750e341bc0d6829a00019435b96830fd4
>
> I can't find this commit. Where is it?
>
> Don't you mean that commit: 28a10c426e81afc88514bca8e73affccf850fdf6
> with title: "net sched: fix encoding to use real length"?
>

Sorry, yes that patch.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH v2] fs/select: add vmalloc fallback for select(2)
From: Vlastimil Babka @ 2016-09-26 10:01 UTC (permalink / raw)
  To: David Laight, Eric Dumazet
  Cc: Alexander Viro, Andrew Morton, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org, Michal Hocko,
	netdev@vger.kernel.org, Linux API, linux-man@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0107FEB@AcuExch.aculab.com>

On 09/23/2016 03:35 PM, David Laight wrote:
> From: Vlastimil Babka
>> Sent: 23 September 2016 10:59
> ...
>> > I suspect that fdt->max_fds is an upper bound for the highest fd the
>> > process has open - not the RLIMIT_NOFILE value.
>>
>> I gathered that the highest fd effectively limits the number of files,
>> so it's the same. I might be wrong.
>
> An application can reduce RLIMIT_NOFILE below that of an open file.

OK, I did some more digging in the code, and my understanding is that:

- fdt->max_fds is the current size of the fdtable, which isn't allocated upfront 
to match the limit, but grows as needed. This means it's OK for 
core_sys_select() to silently cap nfds, as it knows there are no fd's with 
higher number in the fdtable, so it's a performance optimization. However, to 
match what the manpage says, there should be another check against RLIMIT_NOFILE 
to return -EINVAL, which there isn't, AFAICS.

- fdtable is expanded (and fdt->max_fds bumped) by 
expand_files()->expand_fdtable() which checks against fs.nr_open sysctl, which 
seems to be 1048576 where I checked.

- callers of expand_files(), such as dup(), check the rlimit(RLIMIT_NOFILE) to 
limit the expansion.

So yeah, application can reduce RLIMIT_NOFILE, but it has no effect on fdtable 
and fdt->max_fds that is already above the limit. Select syscall would have to 
check the rlimit to conform to the manpage. Or (rather?) we should fix the manpage.

As for the original vmalloc() flood concern, I still think we're safe, as 
ordinary users are limited by RLIMIT_NOFILE way below sizes that would need 
vmalloc(), and root has many other options to DOS the system (or worse).

Vlastimil

> 	David
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v6 5/6] net: ipv4, ipv6: run cgroup ebpf egress programs
From: Daniel Borkmann @ 2016-09-26 10:10 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Thomas Graf, Daniel Mack, htejun, ast, davem, kafai, fw, harald,
	netdev, sargun, cgroups
In-Reply-To: <20160923131739.GA15828@salvia>

On 09/23/2016 03:17 PM, Pablo Neira Ayuso wrote:
> On Thu, Sep 22, 2016 at 05:12:57PM +0200, Daniel Borkmann wrote:
>> On 09/22/2016 02:05 PM, Pablo Neira Ayuso wrote:
> [...]
>>> Have a look at net/ipv4/netfilter/nft_chain_route_ipv4.c for instance.
>>> In your case, you have to add a new chain type:
>>>
>>> static const struct nf_chain_type nft_chain_bpf = {
>>>          .name           = "bpf",
>>>          .type           = NFT_CHAIN_T_bpf,
>>>          ...
>>>          .hooks          = {
>>>                  [NF_INET_LOCAL_IN]      = nft_do_bpf,
>>>                  [NF_INET_LOCAL_OUT]     = nft_do_bpf,
>>>                  [NF_INET_FORWARD]       = nft_do_bpf,
>>>                  [NF_INET_PRE_ROUTING]   = nft_do_bpf,
>>>                  [NF_INET_POST_ROUTING]  = nft_do_bpf,
>>>          },
>>> };
>>>
>>> nft_do_bpf() is the raw netfilter hook that you register, this hook
>>> will just execute to iterate over the list of bpf filters and run
>>> them.
>>>
>>> This new chain is created on demand, so no overhead if not needed, eg.
>>>
>>> nft add table bpf
>>> nft add chain bpf input { type bpf hook output priority 0\; }
>>>
>>> Then, you add a rule for each bpf program you want to run, just like
>>> tc+bpf.
>>
>> But from a high-level point of view, this sounds like a huge hack to me,
>> in the sense that nft as a bytecode engine (from whole architecture I
>> mean) calls into another bytecode engine such as bpf as an extension.
>
> nft is not only bytecode engine, it provides a netlink socket
> interface to register hooks (from user perspective, these are called
> basechain). It is providing the infrastructure that you're lacking
> indeed and addressing the concerns I mentioned about the visibility of
> the global policy that you want to apply on the packet path.
>
> As I explained you can potentially add any basechain type with
> specific semantics. Proposed semantics for this bpf chain would be:
>
> 1) You can use any of the existing netfilter hooks.
> 2) You can only run bpf program from there. No chance for the user
>     can mix nftables with bpf VM.
>
>> And bpf code from there isn't using any of the features from nft
>> besides being invoked from the hook
>
> I think there's a misunderstading here.
>
> You will not run nft_do_chain(), you don't waste cycles to run what is
> specific to nftables. You will just run nft_do_bpf() which will just
> do what you want to run for each packet. Thus, you have control on
> what nft_do_bpf() does and decide on what that function spend cycles
> on.
>
>> [...] I was hoping that nft would try to avoid some of those exotic
>> modules we have from xt, I would consider xt_bpf (no offense ;))
>
> This has nothing to do with it. In xt_bpf you waste cycles running
> code that is specific to iptables, what I propose would not, just the
> generic hook code and then your code.
>
> [...]
>>> Benefits are, rewording previous email:
>>>
>>> * You get access to all of the existing netfilter hooks in one go
>>>    to run bpf programs. No need for specific redundant hooks. This
>>>    provides raw access to the netfilter hook, you define the little
>>>    code that your hook runs before you bpf run invocation. So there
>>>    is *no need to bloat the stack with more hooks, we use what we
>>>    have.*
>>
>> But also this doesn't really address the fundamental underlying problem
>> that is discussed here. nft doesn't even have cgroups v2 support.
>
> You don't need native cgroups v2 support in nft, you just run bpf
> programs from the native bpf basechain type. So whatever bpf supports,
> you can do it.

Yes, and I'm saying that the existing netfilter hooks alone still
don't address the underlying issue as mentioned before. From ingress
side you still need some form of a hook where you get the final sk
(non early-demuxed ones I mean), nothing changed on that issue as far
as I can see.

> Instead, if you take this approach, you will get access to all of the
> existing hooks to run bpf programs, this includes arp, bridge and
> potentially run filters for both ip and ip6 through our inet family.
>
> [...]
>> Or would the idea be that the current netfilter hooks would be redone in
>> a way that they are generic enough so that any other user could make use
>> of it independent of netfilter?
>
> Redone? Why? What do you need, a rename?
>
> Dependencies are very few: CONFIG_NETFILTER for the hooks,
> CONFIG_NF_TABLES to obtain the netlink interface to load the bpf
> programs and CONFIG_NF_TABLES_BPF to define the bpf basechain type
> semantics to run bpf programs from there. It's actually very little
> boilerplate code.

Still not really keen on this idea. You still need an additional,
non-symmetric hook for sk input, we add a dependency that forces
CONFIG_NETFILTER and CONFIG_NF_TABLES where it doesn't really need
it besides the hook point itself, and while boilerplate code for
integrating/adding the BPF basechain may be okay'ish from kernel
side in size, you still need the whole front-end infrastructure
with ELF loader etc we've added over time into tc. So what you would
end up with is the same duplicated infrastructure side you have
with tc + BPF already.

Therefore my question was in the direction of making the hook
generic in a way, where we could just add a new parent id to
sch_clsact for clsact_find_tcf(), and then from all the available
generic hooks, the two needed here can be addressed from sch_clsact
as well without the detour via adding entire infrastructure around
nft, since this is already available from tc by just going via
tc_classify() like we do in functions such as sch_handle_ingress()
and sch_handle_egress(). This still gives you the visibility and
infrastructure you are asking for, and avoids to duplicate the same
functionality from tc over into nft. It would also help interactions
with the already existing 'tc filter add ... {ingress,egress} bpf ..'
parents, since from a BPF programmability perspective, it would
reuse the same existing loader code and thus eases maps sharing,
and allow for program code to be reused in the same object file.

> Other than that, I can predict where you're going: You will end up
> adding a hook just before/after every of the existing netfilter hooks,
> and that is really nonsense to me.

I don't think that is needed here.

Thanks,
Daniel

^ permalink raw reply

* [PATCH] brcmfmac: implement more accurate skb tracking
From: Rafał Miłecki @ 2016-09-26 10:23 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
	Pieter-Paul Giesberts, Franky Lin,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

We need to track 802.1x packets to know if there are any pending ones
for transmission. This is required for performing key update in the
firmware.

Unfortunately our old tracking code wasn't very accurate. It was
treating skb as pending as soon as it was passed by the netif. Actual
handling packet to the firmware was happening later as brcmfmac
internally queues them and uses its own worker(s).

Other than that it was hard to handle freeing packets. Everytime we had
to determine (in more generic funcions) if packet was counted as pending
802.1x one or not. It was causing some problems, e.g. it wasn't clear if
brcmf_flowring_delete should free skb directly or not.

This patch introduces 2 separated functions for tracking skbs. This
simplifies logic, fixes brcmf_flowring_delete (maybe other hidden bugs
as well) and allows further simplifications. Thanks to better accuracy
is also increases time window for key update (and lowers timeout risk).

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
This was successfully tested with 4366b1. Can someone give it a try with
some USB/SDIO device, please?
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcdc.c    | 11 +++++++
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 12 +++++++-
 .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 36 ++++++++++++++++------
 .../wireless/broadcom/brcm80211/brcmfmac/core.h    |  2 ++
 .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c  | 14 +++++++--
 .../wireless/broadcom/brcm80211/brcmfmac/proto.h   | 11 +++++++
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    |  8 +++++
 .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 10 ++++++
 8 files changed, 91 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
index d1bc51f..3e40244 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
@@ -326,6 +326,16 @@ brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws,
 	return 0;
 }
 
+static int brcmf_proto_bcdc_hdr_get_ifidx(struct brcmf_pub *drvr,
+					  struct sk_buff *skb)
+{
+	struct brcmf_proto_bcdc_header *h;
+
+	h = (struct brcmf_proto_bcdc_header *)(skb->data);
+
+	return BCDC_GET_IF_IDX(h);
+}
+
 static int
 brcmf_proto_bcdc_txdata(struct brcmf_pub *drvr, int ifidx, u8 offset,
 			struct sk_buff *pktbuf)
@@ -373,6 +383,7 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr)
 	}
 
 	drvr->proto->hdrpull = brcmf_proto_bcdc_hdrpull;
+	drvr->proto->hdr_get_ifidx = brcmf_proto_bcdc_hdr_get_ifidx;
 	drvr->proto->query_dcmd = brcmf_proto_bcdc_query_dcmd;
 	drvr->proto->set_dcmd = brcmf_proto_bcdc_set_dcmd;
 	drvr->proto->txdata = brcmf_proto_bcdc_txdata;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 03404cb..fef9d02 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -43,6 +43,7 @@
 #include "chip.h"
 #include "bus.h"
 #include "debug.h"
+#include "proto.h"
 #include "sdio.h"
 #include "core.h"
 #include "common.h"
@@ -772,6 +773,7 @@ int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 			 struct sk_buff_head *pktq)
 {
+	struct brcmf_pub *pub = sdiodev->bus_if->drvr;
 	struct sk_buff *skb;
 	u32 addr = sdiodev->sbwad;
 	int err;
@@ -784,10 +786,18 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 
 	if (pktq->qlen == 1 || !sdiodev->sg_support)
 		skb_queue_walk(pktq, skb) {
+			struct brcmf_if *ifp;
+			int ifidx;
+
+			ifidx = brcmf_proto_hdr_get_ifidx(pub, skb);
+			ifp = brcmf_get_ifp(pub, ifidx);
+			brcmf_tx_passing_skb(ifp, skb);
 			err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true,
 						 addr, skb);
-			if (err)
+			if (err) {
+				brcmf_tx_regained_skb(ifp, skb);
 				break;
+			}
 		}
 	else
 		err = brcmf_sdiod_sglist_rw(sdiodev, SDIO_FUNC_2, true, addr,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index bc3d8ab..7cdc1f6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -247,9 +247,6 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
 		goto done;
 	}
 
-	if (eh->h_proto == htons(ETH_P_PAE))
-		atomic_inc(&ifp->pend_8021x_cnt);
-
 	/* determine the priority */
 	if (skb->priority == 0 || skb->priority > 7)
 		skb->priority = cfg80211_classify8021d(skb, NULL);
@@ -388,20 +385,41 @@ void brcmf_rx_event(struct device *dev, struct sk_buff *skb)
 	brcmu_pkt_buf_free_skb(skb);
 }
 
-void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success)
+/**
+ * brcmf_tx_passing_skb - Let core know skb is being passed to the firmware
+ *
+ * Core code needs to track state of some skbs. This function should be called
+ * every time skb is going to be passed to the firmware for transmitting.
+ */
+void brcmf_tx_passing_skb(struct brcmf_if *ifp, struct sk_buff *skb)
 {
-	struct ethhdr *eh;
-	u16 type;
+	struct ethhdr *eh = (struct ethhdr *)(skb->data);
 
-	eh = (struct ethhdr *)(txp->data);
-	type = ntohs(eh->h_proto);
+	if (eh->h_proto == htons(ETH_P_PAE))
+		atomic_inc(&ifp->pend_8021x_cnt);
+}
 
-	if (type == ETH_P_PAE) {
+/**
+ * brcmf_tx_regained_skb - Let core know skb is not being fw processed anymore
+ *
+ * This function should be called every time skb is returned from the firmware
+ * processing for whatever reason. It usually happens after successful
+ * transmission but may be also due to some error.
+ */
+void brcmf_tx_regained_skb(struct brcmf_if *ifp, struct sk_buff *skb)
+{
+	struct ethhdr *eh = (struct ethhdr *)(skb->data);
+
+	if (eh->h_proto == htons(ETH_P_PAE)) {
 		atomic_dec(&ifp->pend_8021x_cnt);
+
 		if (waitqueue_active(&ifp->pend_8021x_wait))
 			wake_up(&ifp->pend_8021x_wait);
 	}
+}
 
+void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success)
+{
 	if (!success)
 		ifp->stats.tx_errors++;
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index f16cfc9..80478b5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -215,6 +215,8 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
 void brcmf_remove_interface(struct brcmf_if *ifp, bool rtnl_locked);
 void brcmf_txflowblock_if(struct brcmf_if *ifp,
 			  enum brcmf_netif_stop_reason reason, bool state);
+void brcmf_tx_passing_skb(struct brcmf_if *ifp, struct sk_buff *skb);
+void brcmf_tx_regained_skb(struct brcmf_if *ifp, struct sk_buff *skb);
 void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
 void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb);
 void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
index 2b9a2bc..2a25eea 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -701,17 +701,22 @@ static void brcmf_msgbuf_txflow(struct brcmf_msgbuf *msgbuf, u16 flowid)
 
 	count = BRCMF_MSGBUF_TX_FLUSH_CNT2 - BRCMF_MSGBUF_TX_FLUSH_CNT1;
 	while (brcmf_flowring_qlen(flow, flowid)) {
+		u8 ifidx = brcmf_flowring_ifidx_get(flow, flowid);
+		struct brcmf_if *ifp = brcmf_get_ifp(msgbuf->drvr, ifidx);
+
 		skb = brcmf_flowring_dequeue(flow, flowid);
 		if (skb == NULL) {
 			brcmf_err("No SKB, but qlen %d\n",
 				  brcmf_flowring_qlen(flow, flowid));
 			break;
 		}
+		brcmf_tx_passing_skb(ifp, skb);
 		skb_orphan(skb);
 		if (brcmf_msgbuf_alloc_pktid(msgbuf->drvr->bus_if->dev,
 					     msgbuf->tx_pktids, skb, ETH_HLEN,
 					     &physaddr, &pktid)) {
 			brcmf_flowring_reinsert(flow, flowid, skb);
+			brcmf_tx_regained_skb(ifp, skb);
 			brcmf_err("No PKTID available !!\n");
 			break;
 		}
@@ -720,6 +725,7 @@ static void brcmf_msgbuf_txflow(struct brcmf_msgbuf *msgbuf, u16 flowid)
 			brcmf_msgbuf_get_pktid(msgbuf->drvr->bus_if->dev,
 					       msgbuf->tx_pktids, pktid);
 			brcmf_flowring_reinsert(flow, flowid, skb);
+			brcmf_tx_regained_skb(ifp, skb);
 			break;
 		}
 		count++;
@@ -728,7 +734,7 @@ static void brcmf_msgbuf_txflow(struct brcmf_msgbuf *msgbuf, u16 flowid)
 
 		tx_msghdr->msg.msgtype = MSGBUF_TYPE_TX_POST;
 		tx_msghdr->msg.request_id = cpu_to_le32(pktid);
-		tx_msghdr->msg.ifidx = brcmf_flowring_ifidx_get(flow, flowid);
+		tx_msghdr->msg.ifidx = ifidx;
 		tx_msghdr->flags = BRCMF_MSGBUF_PKT_FLAGS_FRAME_802_3;
 		tx_msghdr->flags |= (skb->priority & 0x07) <<
 				    BRCMF_MSGBUF_PKT_FLAGS_PRIO_SHIFT;
@@ -857,6 +863,7 @@ brcmf_msgbuf_process_ioctl_complete(struct brcmf_msgbuf *msgbuf, void *buf)
 static void
 brcmf_msgbuf_process_txstatus(struct brcmf_msgbuf *msgbuf, void *buf)
 {
+	struct brcmf_if *ifp;
 	struct brcmf_commonring *commonring;
 	struct msgbuf_tx_status *tx_status;
 	u32 idx;
@@ -876,8 +883,9 @@ brcmf_msgbuf_process_txstatus(struct brcmf_msgbuf *msgbuf, void *buf)
 	commonring = msgbuf->flowrings[flowid];
 	atomic_dec(&commonring->outstanding_tx);
 
-	brcmf_txfinalize(brcmf_get_ifp(msgbuf->drvr, tx_status->msg.ifidx),
-			 skb, true);
+	ifp = brcmf_get_ifp(msgbuf->drvr, tx_status->msg.ifidx);
+	brcmf_tx_regained_skb(ifp, skb);
+	brcmf_txfinalize(ifp, skb, true);
 }
 
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
index 57531f4..453cc5a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
@@ -16,6 +16,7 @@
 #ifndef BRCMFMAC_PROTO_H
 #define BRCMFMAC_PROTO_H
 
+#include "core.h"
 
 enum proto_addr_mode {
 	ADDR_INDIRECT	= 0,
@@ -29,6 +30,7 @@ struct brcmf_skb_reorder_data {
 struct brcmf_proto {
 	int (*hdrpull)(struct brcmf_pub *drvr, bool do_fws,
 		       struct sk_buff *skb, struct brcmf_if **ifp);
+	int (*hdr_get_ifidx)(struct brcmf_pub *drvr, struct sk_buff *skb);
 	int (*query_dcmd)(struct brcmf_pub *drvr, int ifidx, uint cmd,
 			  void *buf, uint len);
 	int (*set_dcmd)(struct brcmf_pub *drvr, int ifidx, uint cmd, void *buf,
@@ -64,6 +66,15 @@ static inline int brcmf_proto_hdrpull(struct brcmf_pub *drvr, bool do_fws,
 		ifp = &tmp;
 	return drvr->proto->hdrpull(drvr, do_fws, skb, ifp);
 }
+
+static inline int brcmf_proto_hdr_get_ifidx(struct brcmf_pub *drvr,
+					    struct sk_buff *skb)
+{
+	if (!drvr->proto->hdr_get_ifidx)
+		return -ENOTSUPP;
+	return drvr->proto->hdr_get_ifidx(drvr, skb);
+}
+
 static inline int brcmf_proto_query_dcmd(struct brcmf_pub *drvr, int ifidx,
 					 uint cmd, void *buf, uint len)
 {
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index b892dac..4dc96bd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -42,6 +42,7 @@
 #include "sdio.h"
 #include "chip.h"
 #include "firmware.h"
+#include "proto.h"
 #include "core.h"
 #include "common.h"
 
@@ -2240,6 +2241,7 @@ brcmf_sdio_txpkt_postp(struct brcmf_sdio *bus, struct sk_buff_head *pktq)
 static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 			    uint chan)
 {
+	struct brcmf_pub *pub = bus->sdiodev->bus_if->drvr;
 	int ret;
 	struct sk_buff *pkt_next, *tmp;
 
@@ -2263,7 +2265,13 @@ done:
 	if (ret == 0)
 		bus->tx_seq = (bus->tx_seq + pktq->qlen) % SDPCM_SEQ_WRAP;
 	skb_queue_walk_safe(pktq, pkt_next, tmp) {
+		struct brcmf_if *ifp;
+		int ifidx;
+
 		__skb_unlink(pkt_next, pktq);
+		ifidx = brcmf_proto_hdr_get_ifidx(pub, pkt_next);
+		ifp = brcmf_get_ifp(pub, ifidx);
+		brcmf_tx_regained_skb(ifp, pkt_next);
 		brcmf_txcomplete(bus->sdiodev->dev, pkt_next, ret == 0);
 	}
 	return ret;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 2f978a3..d2f81c7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -26,6 +26,7 @@
 #include "bus.h"
 #include "debug.h"
 #include "firmware.h"
+#include "proto.h"
 #include "usb.h"
 #include "core.h"
 #include "common.h"
@@ -476,12 +477,16 @@ static void brcmf_usb_tx_complete(struct urb *urb)
 {
 	struct brcmf_usbreq *req = (struct brcmf_usbreq *)urb->context;
 	struct brcmf_usbdev_info *devinfo = req->devinfo;
+	struct brcmf_pub *pub = devinfo->bus_pub.bus->drvr;
+	struct brcmf_if *ifp;
 	unsigned long flags;
 
 	brcmf_dbg(USB, "Enter, urb->status=%d, skb=%p\n", urb->status,
 		  req->skb);
 	brcmf_usb_del_fromq(devinfo, req);
 
+	ifp = brcmf_get_ifp(pub, brcmf_proto_hdr_get_ifidx(pub, req->skb));
+	brcmf_tx_regained_skb(ifp, req->skb);
 	brcmf_txcomplete(devinfo->dev, req->skb, urb->status == 0);
 	req->skb = NULL;
 	brcmf_usb_enq(devinfo, &devinfo->tx_freeq, req, &devinfo->tx_freecount);
@@ -598,7 +603,9 @@ brcmf_usb_state_change(struct brcmf_usbdev_info *devinfo, int state)
 static int brcmf_usb_tx(struct device *dev, struct sk_buff *skb)
 {
 	struct brcmf_usbdev_info *devinfo = brcmf_usb_get_businfo(dev);
+	struct brcmf_pub *pub = devinfo->bus_pub.bus->drvr;
 	struct brcmf_usbreq  *req;
+	struct brcmf_if *ifp;
 	int ret;
 	unsigned long flags;
 
@@ -622,6 +629,8 @@ static int brcmf_usb_tx(struct device *dev, struct sk_buff *skb)
 			  skb->data, skb->len, brcmf_usb_tx_complete, req);
 	req->urb->transfer_flags |= URB_ZERO_PACKET;
 	brcmf_usb_enq(devinfo, &devinfo->tx_postq, req, NULL);
+	ifp = brcmf_get_ifp(pub, brcmf_proto_hdr_get_ifidx(pub, skb));
+	brcmf_tx_passing_skb(ifp, skb);
 	ret = usb_submit_urb(req->urb, GFP_ATOMIC);
 	if (ret) {
 		brcmf_err("brcmf_usb_tx usb_submit_urb FAILED\n");
@@ -629,6 +638,7 @@ static int brcmf_usb_tx(struct device *dev, struct sk_buff *skb)
 		req->skb = NULL;
 		brcmf_usb_enq(devinfo, &devinfo->tx_freeq, req,
 			      &devinfo->tx_freecount);
+		brcmf_tx_regained_skb(ifp, skb);
 		goto fail;
 	}
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH net v3 0/2] Fix tc-ife bugs
From: Yotam Gigi @ 2016-09-26 10:45 UTC (permalink / raw)
  To: jhs, davem, netdev, mlxsw; +Cc: Yotam Gigi

This patch-set contains two bugfixes in the tc-ife action, one fixing some
random behaviour in encode side, and one fixing the decode side packet
parsing logic.

v2->v3
 - Fix the encode side instead of the decode side

Yotam Gigi (2):
  act_ife: Fix external mac header on encode
  act_ife: Fix false encoding

 net/sched/act_ife.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.4.11

^ permalink raw reply

* [PATCH net v3 1/2] act_ife: Fix external mac header on encode
From: Yotam Gigi @ 2016-09-26 10:45 UTC (permalink / raw)
  To: jhs, davem, netdev, mlxsw; +Cc: Yotam Gigi
In-Reply-To: <1474886726-56363-1-git-send-email-yotamg@mellanox.com>

On ife encode side, external mac header is copied from the original packet
and may be overridden if the user requests. Before, the mac header copy
was done from memory region that might not be accessible anymore, as
skb_cow_head might free it and copy the packet. This led to random values
in the external mac header once the values were not set by user.

This fix takes the internal mac header from the packet, after the call to
skb_cow_head.

Fixes: ef6980b6becb ("net sched: introduce IFE action")
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
---
 net/sched/act_ife.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index ccf7b4b..b949d97 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -766,8 +766,6 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 		return TC_ACT_SHOT;
 	}
 
-	iethh = eth_hdr(skb);
-
 	err = skb_cow_head(skb, hdrm);
 	if (unlikely(err)) {
 		ife->tcf_qstats.drops++;
@@ -778,6 +776,7 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	if (!(at & AT_EGRESS))
 		skb_push(skb, skb->dev->hard_header_len);
 
+	iethh = (struct ethhdr *)skb->data;
 	__skb_push(skb, hdrm);
 	memcpy(skb->data, iethh, skb->mac_len);
 	skb_reset_mac_header(skb);
-- 
2.4.11

^ permalink raw reply related

* [PATCH net v3 2/2] act_ife: Fix false encoding
From: Yotam Gigi @ 2016-09-26 10:45 UTC (permalink / raw)
  To: jhs, davem, netdev, mlxsw; +Cc: Yotam Gigi
In-Reply-To: <1474886726-56363-1-git-send-email-yotamg@mellanox.com>

On ife encode side, the action stores the different tlvs inside the ife
header, where each tlv length field should refer to the length of the
whole tlv (without additional padding) and not just the data length.

On ife decode side, the action iterates over the tlvs in the ife header
and parses them one by one, where in each iteration the current pointer is
advanced according to the tlv size.

Before, the encoding encoded only the data length inside the tlv, which led
to false parsing of ife the header. In addition, due to the fact that the
loop counter was unsigned, it could lead to infinite parsing loop.

This fix changes the loop counter to be signed and fixes the encoding to
take into account the tlv type and size.

Fixes: 28a10c426e81 ("net sched: fix encoding to use real length")
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
---

v2->v3
 - Fix the encode side instead of the decode side

---
 net/sched/act_ife.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index b949d97..95c463c 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -53,7 +53,7 @@ int ife_tlv_meta_encode(void *skbdata, u16 attrtype, u16 dlen, const void *dval)
 	u32 *tlv = (u32 *)(skbdata);
 	u16 totlen = nla_total_size(dlen);	/*alignment + hdr */
 	char *dptr = (char *)tlv + NLA_HDRLEN;
-	u32 htlv = attrtype << 16 | dlen;
+	u32 htlv = attrtype << 16 | (dlen + NLA_HDRLEN);
 
 	*tlv = htonl(htlv);
 	memset(dptr, 0, totlen - NLA_HDRLEN);
@@ -653,7 +653,7 @@ static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
 	struct tcf_ife_info *ife = to_ife(a);
 	int action = ife->tcf_action;
 	struct ifeheadr *ifehdr = (struct ifeheadr *)skb->data;
-	u16 ifehdrln = ifehdr->metalen;
+	int ifehdrln = (int)ifehdr->metalen;
 	struct meta_tlvhdr *tlv = (struct meta_tlvhdr *)(ifehdr->tlv_data);
 
 	spin_lock(&ife->tcf_lock);
-- 
2.4.11

^ permalink raw reply related

* [patch net-next v3 0/6] fib offload: switch to notifier
From: Jiri Pirko @ 2016-09-26 10:52 UTC (permalink / raw)
  To: netdev
  Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz, roopa, nikolay,
	linville, andy, f.fainelli, dsa, jhs, vivien.didelot, andrew,
	ivecera, kaber, john

From: Jiri Pirko <jiri@mellanox.com>

The goal of this patchset is to allow driver to propagate all prefixes
configured in kernel down HW. This is necessary for routing to work
as expected. If we don't do that HW might forward prefixes known to kernel
incorrectly. Take an example when default route is set in switch HW and there
is an IP address set on a management (non-switch) port.

Currently, only FIB entries related to the switch port netdev are
offloaded using switchdev ops. This model is not extendable so the
first patch introduces a replacement: notifier to propagate FIB entry
additions and removals to whoever is interested.

The second patch introduces couple of helpers to deal with RTNH_F_OFFLOAD
flags. Currently it is set in switchdev core. There the assumption is
that only one offload device exists. But for FIB notifier, we assume
multiple offload devices. So the patch introduces a per FIB entry
reference counter and helpers use it in order to achieve this:
   0 means RTNH_F_OFFLOAD is not set, no device offloads this entry
   n means RTNH_F_OFFLOAD is set and the entry is offloaded by n devices

Patches 3 and 4 convert mlxsw and rocker to adopt this new way, registering
one notifier block for each asic instance. Both of these patches also
implement internal "abort" mechanism.

Using switchdev ops, "abort" is called by switchdev core whenever there is
an error during FIB entry add offload. This leads to removal of all
offloaded entries on system by fib_trie code.

Now the new notifier assumes the driver takes care of the abort action.
Here's why:
1) The fact that one HW cannot offload an entry does not mean that the
   others can't do it. So let only one entity to abort and leave the rest
   to work happily.
2) The driver knows what to in order to properly abort. For example,
   currently abort is broken for mlxsw, as for Spectrum there is a need
   to set 0.0.0.0/0 trap in RALUE register.

The fifth patch removes the old, no longer used FIB offload infrastructure.

The last patch reflects the changes into switchdev documentation file.

---
v2->v3:
 -patch 3/6
   -fixed offload inc/dec to be done in fib4_entry_init/fini and only
    in case !trap as suggested by Ido
v1->v2:
 -patch 3/6:
   -fixed lpm tree setup and binding for abort and pointed out by Ido
   -do nexthop checks as suggested by Ido
   -fix use after free during abort
 -patch 6/6:
   -fixed texts as suggested by Ido

Jiri Pirko (6):
  fib: introduce FIB notification infrastructure
  fib: introduce FIB info offload flag helpers
  mlxsw: spectrum_router: Use FIB notifications instead of switchdev
    calls
  rocker: use FIB notifications instead of switchdev calls
  switchdev: remove FIB offload infrastructure
  doc: update switchdev L3 section

 Documentation/networking/switchdev.txt             |  27 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |   9 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 437 ++++++++++++---------
 .../ethernet/mellanox/mlxsw/spectrum_switchdev.c   |   9 -
 drivers/net/ethernet/rocker/rocker.h               |  15 +-
 drivers/net/ethernet/rocker/rocker_main.c          | 120 ++++--
 drivers/net/ethernet/rocker/rocker_ofdpa.c         | 115 ++++--
 include/net/ip_fib.h                               |  49 ++-
 include/net/switchdev.h                            |  40 --
 net/ipv4/fib_frontend.c                            |  29 +-
 net/ipv4/fib_rules.c                               |  12 +-
 net/ipv4/fib_trie.c                                | 166 +++-----
 net/switchdev/switchdev.c                          | 181 ---------
 13 files changed, 581 insertions(+), 628 deletions(-)

-- 
2.5.5

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox