Netdev List
 help / color / mirror / Atom feed
* [net 3/3] gianfar: Refactor link flow control update (adjust_link)
From: Claudiu Manoil @ 2017-09-01  9:41 UTC (permalink / raw)
  To: netdev; +Cc: David S . Miller
In-Reply-To: <1504258863-2058-1-git-send-email-claudiu.manoil@nxp.com>

Encapsulate link layer flow control logic. These settings
are touching maccfg1 reg exclusively.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 68 ++++++++++++++++----------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 46880a9..1648173 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -3672,6 +3672,36 @@ static u32 gfar_get_flowctrl_cfg(struct gfar_private *priv)
 	return val;
 }
 
+static void gfar_update_link_flowctrl(struct gfar_private *priv)
+{
+	struct gfar __iomem *regs = priv->gfargrp[0].regs;
+	u32 maccfg1, tx_flow_oldval;
+	int i;
+
+	maccfg1 = gfar_read(&regs->maccfg1);
+	tx_flow_oldval = (maccfg1 & MACCFG1_TX_FLOW);
+
+	maccfg1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW);
+	maccfg1 |= gfar_get_flowctrl_cfg(priv);
+
+	/* Turn last free buffer recording on */
+	if ((maccfg1 & MACCFG1_TX_FLOW) && !tx_flow_oldval) {
+		for (i = 0; i < priv->num_rx_queues; i++) {
+			u32 bdp_dma;
+
+			bdp_dma = gfar_rxbd_dma_lastfree(priv->rx_queue[i]);
+			gfar_write(priv->rx_queue[i]->rfbptr, bdp_dma);
+		}
+
+		priv->tx_actual_en = 1;
+	}
+
+	if (unlikely(!(maccfg1 & MACCFG1_TX_FLOW) && tx_flow_oldval))
+		priv->tx_actual_en = 0;
+
+	gfar_write(&regs->maccfg1, maccfg1);
+}
+
 static void gfar_update_link_speed(struct gfar_private *priv)
 {
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
@@ -3722,44 +3752,14 @@ static void gfar_update_link_speed(struct gfar_private *priv)
 
 static noinline void gfar_update_link_state(struct gfar_private *priv)
 {
-	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	struct net_device *ndev = priv->ndev;
-	struct phy_device *phydev = ndev->phydev;
-	struct gfar_priv_rx_q *rx_queue = NULL;
-	int i;
-
 	if (unlikely(test_bit(GFAR_RESETTING, &priv->state)))
 		return;
 
-	if (phydev->link) {
-		u32 tempval1 = gfar_read(&regs->maccfg1);
-		u32 tx_flow_oldval = (tempval1 & MACCFG1_TX_FLOW);
-
+	if (priv->ndev->phydev->link) {
+		gfar_update_link_flowctrl(priv);
 		gfar_update_link_speed(priv);
 
-		tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW);
-		tempval1 |= gfar_get_flowctrl_cfg(priv);
-
-		/* Turn last free buffer recording on */
-		if ((tempval1 & MACCFG1_TX_FLOW) && !tx_flow_oldval) {
-			for (i = 0; i < priv->num_rx_queues; i++) {
-				u32 bdp_dma;
-
-				rx_queue = priv->rx_queue[i];
-				bdp_dma = gfar_rxbd_dma_lastfree(rx_queue);
-				gfar_write(rx_queue->rfbptr, bdp_dma);
-			}
-
-			priv->tx_actual_en = 1;
-		}
-
-		if (unlikely(!(tempval1 & MACCFG1_TX_FLOW) && tx_flow_oldval))
-			priv->tx_actual_en = 0;
-
-		gfar_write(&regs->maccfg1, tempval1);
-
-		if (!priv->oldlink)
-			priv->oldlink = 1;
+		priv->oldlink = 1;
 
 	} else if (priv->oldlink) {
 		priv->oldlink = 0;
@@ -3768,7 +3768,7 @@ static noinline void gfar_update_link_state(struct gfar_private *priv)
 	}
 
 	if (netif_msg_link(priv))
-		phy_print_status(phydev);
+		phy_print_status(priv->ndev->phydev);
 }
 
 static const struct of_device_id gfar_match[] =
-- 
2.7.4

^ permalink raw reply related
* Re: [PATCH net-next v7 08/10] bpf: Add a Landlock sandbox example
From: Alban Crequy @ 2017-09-01 10:25 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel@vger.kernel.org, Alexei Starovoitov, Andy Lutomirski,
	Arnaldo Carvalho de Melo, Casey Schaufler, Daniel Borkmann,
	David Drysdale, David S . Miller, Eric W . Biederman,
	James Morris, Jann Horn, Jonathan Corbet, Matthew Garrett,
	Michael Kerrisk, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Tejun 
In-Reply-To: <20170821000933.13024-9-mic@digikod.net>

Hi Mickaël,

On 21 August 2017 at 02:09, Mickaël Salaün <mic@digikod.net> wrote:
> Add a basic sandbox tool to create a process isolated from some part of
> the system. This sandbox create a read-only environment. It is only
> allowed to write to a character device such as a TTY:
...
> +       /*
> +        * This check allows the action on the file if it is a directory or a
> +        * pipe. Otherwise, a message is printed to the eBPF log.
> +        */
> +       if (S_ISCHR(ret) || S_ISFIFO(ret))
> +               return 0;


The comment says "directory", but the code checks for "character device".

Thanks!
Alban

^ permalink raw reply
* [net-next PATCH] ixgbe: add counter for times rx pages gets allocated, not recycled
From: Jesper Dangaard Brouer @ 2017-09-01 10:54 UTC (permalink / raw)
  To: netdev; +Cc: Jeff Kirsher, Alexander Duyck, Jesper Dangaard Brouer

The ixgbe driver have page recycle scheme based around the RX-ring
queue, where a RX page is shared between two packets. Based on the
refcnt, the driver can determine if the RX-page is currently only used
by a single packet, if so it can then directly refill/recycle the
RX-slot by with the opposite "side" of the page.

While this is a clever trick, it is hard to determine when this
recycling is successful and when it fails.  Adding a counter, which is
available via ethtool --statistics as 'alloc_rx_page'.  Which counts
the number of times the recycle fails and the real page allocator is
invoked.  When interpreting the stats, do remember that every alloc
will serve two packets.

The counter is collected per rx_ring, but is summed and ethtool
exported as 'alloc_rx_page'.  It would be relevant to know what
rx_ring that cannot keep up, but that can be exported later if
someone experience a need for this.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h         |    2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c |    1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    |    4 ++++
 3 files changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index dd5578756ae0..008d0085e01f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -275,6 +275,7 @@ struct ixgbe_rx_queue_stats {
 	u64 rsc_count;
 	u64 rsc_flush;
 	u64 non_eop_descs;
+	u64 alloc_rx_page;
 	u64 alloc_rx_page_failed;
 	u64 alloc_rx_buff_failed;
 	u64 csum_err;
@@ -655,6 +656,7 @@ struct ixgbe_adapter {
 	u64 rsc_total_count;
 	u64 rsc_total_flush;
 	u64 non_eop_descs;
+	u32 alloc_rx_page;
 	u32 alloc_rx_page_failed;
 	u32 alloc_rx_buff_failed;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 72c565712a5f..d96d9d6c3492 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -104,6 +104,7 @@ static const struct ixgbe_stats ixgbe_gstrings_stats[] = {
 	{"tx_flow_control_xoff", IXGBE_STAT(stats.lxofftxc)},
 	{"rx_flow_control_xoff", IXGBE_STAT(stats.lxoffrxc)},
 	{"rx_csum_offload_errors", IXGBE_STAT(hw_csum_rx_error)},
+	{"alloc_rx_page", IXGBE_STAT(alloc_rx_page)},
 	{"alloc_rx_page_failed", IXGBE_STAT(alloc_rx_page_failed)},
 	{"alloc_rx_buff_failed", IXGBE_STAT(alloc_rx_buff_failed)},
 	{"rx_no_dma_resources", IXGBE_STAT(hw_rx_no_dma_resources)},
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d962368d08d0..7d2e4b08cdf4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1598,6 +1598,7 @@ static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
 		rx_ring->rx_stats.alloc_rx_page_failed++;
 		return false;
 	}
+	rx_ring->rx_stats.alloc_rx_page++;
 
 	/* map page for use */
 	dma = dma_map_page_attrs(rx_ring->dev, page, 0,
@@ -6771,6 +6772,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
 	u32 i, missed_rx = 0, mpc, bprc, lxon, lxoff, xon_off_tot;
 	u64 non_eop_descs = 0, restart_queue = 0, tx_busy = 0;
 	u64 alloc_rx_page_failed = 0, alloc_rx_buff_failed = 0;
+	u64 alloc_rx_page = 0;
 	u64 bytes = 0, packets = 0, hw_csum_rx_error = 0;
 
 	if (test_bit(__IXGBE_DOWN, &adapter->state) ||
@@ -6791,6 +6793,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
 	for (i = 0; i < adapter->num_rx_queues; i++) {
 		struct ixgbe_ring *rx_ring = adapter->rx_ring[i];
 		non_eop_descs += rx_ring->rx_stats.non_eop_descs;
+		alloc_rx_page += rx_ring->rx_stats.alloc_rx_page;
 		alloc_rx_page_failed += rx_ring->rx_stats.alloc_rx_page_failed;
 		alloc_rx_buff_failed += rx_ring->rx_stats.alloc_rx_buff_failed;
 		hw_csum_rx_error += rx_ring->rx_stats.csum_err;
@@ -6798,6 +6801,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
 		packets += rx_ring->stats.packets;
 	}
 	adapter->non_eop_descs = non_eop_descs;
+	adapter->alloc_rx_page = alloc_rx_page;
 	adapter->alloc_rx_page_failed = alloc_rx_page_failed;
 	adapter->alloc_rx_buff_failed = alloc_rx_buff_failed;
 	adapter->hw_csum_rx_error = hw_csum_rx_error;

^ permalink raw reply related
* Re: [PATCH net] bridge: switchdev: Clear forward mark when transmitting packet
From: Nikolay Aleksandrov @ 2017-09-01 11:45 UTC (permalink / raw)
  To: Ido Schimmel, netdev; +Cc: davem, stephen, jiri, yotamg, mlxsw, bridge
In-Reply-To: <20170901092225.31597-1-idosch@mellanox.com>

On 01/09/17 12:22, Ido Schimmel wrote:
> Commit 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for
> stacked devices") added the 'offload_fwd_mark' bit to the skb in order
> to allow drivers to indicate to the bridge driver that they already
> forwarded the packet in L2.
> 
> In case the bit is set, before transmitting the packet from each port,
> the port's mark is compared with the mark stored in the skb's control
> block. If both marks are equal, we know the packet arrived from a switch
> device that already forwarded the packet and it's not re-transmitted.
> 
> However, if the packet is transmitted from the bridge device itself
> (e.g., br0), we should clear the 'offload_fwd_mark' bit as the mark
> stored in the skb's control block isn't valid.
> 
> This scenario can happen in rare cases where a packet was trapped during
> L3 forwarding and forwarded by the kernel to a bridge device.
> 
> Fixes: 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for stacked devices")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: Yotam Gigi <yotamg@mellanox.com>
> Tested-by: Yotam Gigi <yotamg@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  net/bridge/br_device.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index 861ae2a165f4..5a7be3bddfa9 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -53,6 +53,9 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>  	brstats->tx_bytes += skb->len;
>  	u64_stats_update_end(&brstats->syncp);
>  
> +#ifdef CONFIG_NET_SWITCHDEV
> +	skb->offload_fwd_mark = 0;
> +#endif
>  	BR_INPUT_SKB_CB(skb)->brdev = dev;
>  
>  	skb_reset_mac_header(skb);
> 

Good catch, just one minor nit since there is already an ifdef
switchdev/else in br_private.h, why not make this a helper and avoid the
ifdef/endif in here ? Currently there is no ifdef switchdev anywhere else.

Thanks,
 Nik

^ permalink raw reply
* [PATCH v2 0/5] net: mdio-mux: Misc fix
From: Corentin Labbe @ 2017-09-01 11:55 UTC (permalink / raw)
  To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Corentin Labbe

Hello

This patch series fix minor problems found when working on the
dwmac-sun8i syscon mdio-mux.

Regards

Changes since v1:
- Removed obsolete comment about of_mdio_find_bus/put_device
- removed more DRV_VERSION

Corentin Labbe (5):
  net: mdio-mux: Fix NULL Comparison style
  net: mdio-mux: Remove unnecessary 'out of memory' message
  net: mdio-mux: printing driver version is useless
  net: mdio-mux-mmioreg: Can handle 8/16/32 bits registers
  net: mdio-mux: fix unbalanced put_device

 drivers/net/phy/Kconfig    |  2 +-
 drivers/net/phy/mdio-mux.c | 19 ++++---------------
 2 files changed, 5 insertions(+), 16 deletions(-)

-- 
2.13.5

^ permalink raw reply
* [PATCH v2 1/5] net: mdio-mux: Fix NULL Comparison style
From: Corentin Labbe @ 2017-09-01 11:56 UTC (permalink / raw)
  To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Corentin Labbe
In-Reply-To: <20170901115604.27513-1-clabbe.montjoie@gmail.com>

This patch fix checkpatch warning about NULL Comparison style.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/mdio-mux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 942ceaf3fd3f..b18ad7082b88 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -120,7 +120,7 @@ int mdio_mux_init(struct device *dev,
 	}
 
 	pb = devm_kzalloc(dev, sizeof(*pb), GFP_KERNEL);
-	if (pb == NULL) {
+	if (!pb) {
 		ret_val = -ENOMEM;
 		goto err_pb_kz;
 	}
@@ -144,7 +144,7 @@ int mdio_mux_init(struct device *dev,
 		}
 
 		cb = devm_kzalloc(dev, sizeof(*cb), GFP_KERNEL);
-		if (cb == NULL) {
+		if (!cb) {
 			dev_err(dev,
 				"Error: Failed to allocate memory for child %pOF\n",
 				child_bus_node);
-- 
2.13.5

^ permalink raw reply related
* [PATCH v2 3/5] net: mdio-mux: printing driver version is useless
From: Corentin Labbe @ 2017-09-01 11:56 UTC (permalink / raw)
  To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Corentin Labbe
In-Reply-To: <20170901115604.27513-1-clabbe.montjoie@gmail.com>

Remove the driver version information because this information
is not useful in an upstream kernel driver.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/phy/mdio-mux.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 5e08e89465c5..282828551bdd 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -13,7 +13,6 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 
-#define DRV_VERSION "1.0"
 #define DRV_DESCRIPTION "MDIO bus multiplexer driver"
 
 struct mdio_mux_child_bus;
@@ -179,7 +178,6 @@ int mdio_mux_init(struct device *dev,
 	}
 	if (pb->children) {
 		*mux_handle = pb;
-		dev_info(dev, "Version " DRV_VERSION "\n");
 		return 0;
 	}
 
@@ -212,6 +210,5 @@ void mdio_mux_uninit(void *mux_handle)
 EXPORT_SYMBOL_GPL(mdio_mux_uninit);
 
 MODULE_DESCRIPTION(DRV_DESCRIPTION);
-MODULE_VERSION(DRV_VERSION);
 MODULE_AUTHOR("David Daney");
 MODULE_LICENSE("GPL");
-- 
2.13.5

^ permalink raw reply related
* [PATCH v2 4/5] net: mdio-mux-mmioreg: Can handle 8/16/32 bits registers
From: Corentin Labbe @ 2017-09-01 11:56 UTC (permalink / raw)
  To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Corentin Labbe
In-Reply-To: <20170901115604.27513-1-clabbe.montjoie@gmail.com>

This patch fix an old information that mdio-mux-mmioreg can only handle
8bit registers.
This is not true anymore.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 5afe6fdcc968..a9d16a3af514 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -85,7 +85,7 @@ config MDIO_BUS_MUX_MMIOREG
 	  parent bus.  Child bus selection is under the control of one of
 	  the FPGA's registers.
 
-	  Currently, only 8-bit registers are supported.
+	  Currently, only 8/16/32 bits registers are supported.
 
 config MDIO_CAVIUM
 	tristate
-- 
2.13.5

^ permalink raw reply related
* [PATCH v2 5/5] net: mdio-mux: fix unbalanced put_device
From: Corentin Labbe @ 2017-09-01 11:56 UTC (permalink / raw)
  To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Corentin Labbe
In-Reply-To: <20170901115604.27513-1-clabbe.montjoie@gmail.com>

mdio_mux_uninit() call put_device (unconditionally) because of
of_mdio_find_bus() in mdio_mux_init.
But of_mdio_find_bus is only called if mux_bus is empty.
If mux_bus is set, mdio_mux_uninit will print a "refcount_t: underflow"
trace.

This patch add a get_device in the other branch of "if (mux_bus)".

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/phy/mdio-mux.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 282828551bdd..6f75e9f27fed 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -116,6 +116,7 @@ int mdio_mux_init(struct device *dev,
 	} else {
 		parent_bus_node = NULL;
 		parent_bus = mux_bus;
+		get_device(&parent_bus->dev);
 	}
 
 	pb = devm_kzalloc(dev, sizeof(*pb), GFP_KERNEL);
@@ -184,9 +185,7 @@ int mdio_mux_init(struct device *dev,
 	dev_err(dev, "Error: No acceptable child buses found\n");
 	devm_kfree(dev, pb);
 err_pb_kz:
-	/* balance the reference of_mdio_find_bus() took */
-	if (!mux_bus)
-		put_device(&parent_bus->dev);
+	put_device(&parent_bus->dev);
 err_parent_bus:
 	of_node_put(parent_bus_node);
 	return ret_val;
@@ -204,7 +203,6 @@ void mdio_mux_uninit(void *mux_handle)
 		cb = cb->next;
 	}
 
-	/* balance the reference of_mdio_find_bus() in mdio_mux_init() took */
 	put_device(&pb->mii_bus->dev);
 }
 EXPORT_SYMBOL_GPL(mdio_mux_uninit);
-- 
2.13.5

^ permalink raw reply related
* [PATCH v2 2/5] net: mdio-mux: Remove unnecessary 'out of memory' message
From: Corentin Labbe @ 2017-09-01 11:56 UTC (permalink / raw)
  To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Corentin Labbe
In-Reply-To: <20170901115604.27513-1-clabbe.montjoie@gmail.com>

This patch fix checkpatch warning about unnecessary 'out of memory'
message.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/mdio-mux.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index b18ad7082b88..5e08e89465c5 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -145,9 +145,6 @@ int mdio_mux_init(struct device *dev,
 
 		cb = devm_kzalloc(dev, sizeof(*cb), GFP_KERNEL);
 		if (!cb) {
-			dev_err(dev,
-				"Error: Failed to allocate memory for child %pOF\n",
-				child_bus_node);
 			ret_val = -ENOMEM;
 			continue;
 		}
@@ -156,9 +153,6 @@ int mdio_mux_init(struct device *dev,
 
 		cb->mii_bus = mdiobus_alloc();
 		if (!cb->mii_bus) {
-			dev_err(dev,
-				"Error: Failed to allocate MDIO bus for child %pOF\n",
-				child_bus_node);
 			ret_val = -ENOMEM;
 			devm_kfree(dev, cb);
 			continue;
-- 
2.13.5

^ permalink raw reply related
* Re: [PATCH net] bridge: switchdev: Clear forward mark when transmitting packet
From: Jiri Pirko @ 2017-09-01 12:01 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Ido Schimmel, netdev, davem, stephen, jiri, yotamg, mlxsw, bridge
In-Reply-To: <0384baf6-0786-2d07-8919-fdd1ca5d88eb@cumulusnetworks.com>

Fri, Sep 01, 2017 at 01:45:15PM CEST, nikolay@cumulusnetworks.com wrote:
>On 01/09/17 12:22, Ido Schimmel wrote:
>> Commit 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for
>> stacked devices") added the 'offload_fwd_mark' bit to the skb in order
>> to allow drivers to indicate to the bridge driver that they already
>> forwarded the packet in L2.
>> 
>> In case the bit is set, before transmitting the packet from each port,
>> the port's mark is compared with the mark stored in the skb's control
>> block. If both marks are equal, we know the packet arrived from a switch
>> device that already forwarded the packet and it's not re-transmitted.
>> 
>> However, if the packet is transmitted from the bridge device itself
>> (e.g., br0), we should clear the 'offload_fwd_mark' bit as the mark
>> stored in the skb's control block isn't valid.
>> 
>> This scenario can happen in rare cases where a packet was trapped during
>> L3 forwarding and forwarded by the kernel to a bridge device.
>> 
>> Fixes: 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for stacked devices")
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> Reported-by: Yotam Gigi <yotamg@mellanox.com>
>> Tested-by: Yotam Gigi <yotamg@mellanox.com>
>> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>  net/bridge/br_device.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
>> index 861ae2a165f4..5a7be3bddfa9 100644
>> --- a/net/bridge/br_device.c
>> +++ b/net/bridge/br_device.c
>> @@ -53,6 +53,9 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>>  	brstats->tx_bytes += skb->len;
>>  	u64_stats_update_end(&brstats->syncp);
>>  
>> +#ifdef CONFIG_NET_SWITCHDEV
>> +	skb->offload_fwd_mark = 0;
>> +#endif
>>  	BR_INPUT_SKB_CB(skb)->brdev = dev;
>>  
>>  	skb_reset_mac_header(skb);
>> 
>
>Good catch, just one minor nit since there is already an ifdef
>switchdev/else in br_private.h, why not make this a helper and avoid the
>ifdef/endif in here ? Currently there is no ifdef switchdev anywhere else.

I think it would be better to convert this to a helper in -net-next and
take the patch as it is for -net

^ permalink raw reply
* Re: virtio_net: ethtool supported link modes
From: Radu Rendec @ 2017-09-01 12:01 UTC (permalink / raw)
  To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization, netdev, linux-kernel
In-Reply-To: <38286395-683f-f285-aaa5-68f0e8b68675@redhat.com>

On Fri, 2017-09-01 at 11:36 +0800, Jason Wang wrote:
> 
> On 2017年09月01日 01:04, Radu Rendec wrote:
> > Hello,
> > 
> > Looking at the code in virtnet_set_link_ksettings, it seems the speed
> > and duplex can be set to any valid value. The driver will "remember"
> > them and report them back in virtnet_get_link_ksettings.
> > 
> > However, the supported link modes (link_modes.supported in struct
> > ethtool_link_ksettings) is always 0, indicating that no speed/duplex
> > setting is supported.
> > 
> > Does it make more sense to set (at least a few of) the supported link
> > modes, such as 10baseT_Half ... 10000baseT_Full?
> > 
> > I would expect to see consistency between what is reported in
> > link_modes.supported and what can actually be set. Could you please
> > share your opinion on this?
> 
> I think the may make sense only if there's a hardware implementation for 
> virtio. And we probably need to extend virtio spec for adding new commands.

So you're saying that the "hardware" should provide the supported link
modes (e.g. by using feature bits at the virtio layer) and the
virtio_net driver should just translate them and expose them as
link_modes.supported?

Then for consistency, I assume setting speed/duplex via ethtool should
also go into the virtio layer (currently virtio_net seems to just store
them for future retrieval via ethtool).

Thanks,
Radu

^ permalink raw reply
* Re: [PATCH net] bridge: switchdev: Clear forward mark when transmitting packet
From: Nikolay Aleksandrov @ 2017-09-01 12:03 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: mlxsw, yotamg, netdev, bridge, Ido Schimmel, jiri, davem
In-Reply-To: <20170901120120.GA1962@nanopsycho>

On 01/09/17 15:01, Jiri Pirko wrote:
> Fri, Sep 01, 2017 at 01:45:15PM CEST, nikolay@cumulusnetworks.com wrote:
>> On 01/09/17 12:22, Ido Schimmel wrote:
>>> Commit 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for
>>> stacked devices") added the 'offload_fwd_mark' bit to the skb in order
>>> to allow drivers to indicate to the bridge driver that they already
>>> forwarded the packet in L2.
>>>
>>> In case the bit is set, before transmitting the packet from each port,
>>> the port's mark is compared with the mark stored in the skb's control
>>> block. If both marks are equal, we know the packet arrived from a switch
>>> device that already forwarded the packet and it's not re-transmitted.
>>>
>>> However, if the packet is transmitted from the bridge device itself
>>> (e.g., br0), we should clear the 'offload_fwd_mark' bit as the mark
>>> stored in the skb's control block isn't valid.
>>>
>>> This scenario can happen in rare cases where a packet was trapped during
>>> L3 forwarding and forwarded by the kernel to a bridge device.
>>>
>>> Fixes: 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for stacked devices")
>>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>>> Reported-by: Yotam Gigi <yotamg@mellanox.com>
>>> Tested-by: Yotam Gigi <yotamg@mellanox.com>
>>> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>>> ---
>>>  net/bridge/br_device.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
>>> index 861ae2a165f4..5a7be3bddfa9 100644
>>> --- a/net/bridge/br_device.c
>>> +++ b/net/bridge/br_device.c
>>> @@ -53,6 +53,9 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>>>  	brstats->tx_bytes += skb->len;
>>>  	u64_stats_update_end(&brstats->syncp);
>>>  
>>> +#ifdef CONFIG_NET_SWITCHDEV
>>> +	skb->offload_fwd_mark = 0;
>>> +#endif
>>>  	BR_INPUT_SKB_CB(skb)->brdev = dev;
>>>  
>>>  	skb_reset_mac_header(skb);
>>>
>>
>> Good catch, just one minor nit since there is already an ifdef
>> switchdev/else in br_private.h, why not make this a helper and avoid the
>> ifdef/endif in here ? Currently there is no ifdef switchdev anywhere else.
> 
> I think it would be better to convert this to a helper in -net-next and
> take the patch as it is for -net
> 

Either way is fine I guess, it's just more work for something as simple. :-)

Whichever way you choose,

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

^ permalink raw reply
* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Jan Scheurich @ 2017-09-01 12:11 UTC (permalink / raw)
  To: Hannes Frederic Sowa, Mooney, Sean K
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, e@erig.me,
	jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <87inh56q8u.fsf-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>

> > [Mooney, Sean K]
> > Having the nsh context headers in the flow is quite useful It would
> > allow loadblancing on values stored in the context headers Or other
> > use. I belive odl previously used context header 4 to store a Flow id
> > so this could potentialy be used with the multipath action to have ovs
> > Choose between several possible next hops in the chain.
> 
> In OVS, masks are a list(!) for matching. How can this work for different
> paths that might require different masks? If they can't be unified you even
> get exact matches. Thus, for OVS the context should not be part of the
> flow.

The NSH support in OVS 2.8 (for the user-space datapath only, so far) supports matching on and manipulating the fixed size MD1 context headers C1-C4. They are part of the flow and there are corresponding OXM fields defined. It is up to the SDN controller to program pipelines that match on or set these fields. 

The goal was to support all relevant NSH use cases for MD1: Classifier, SFF, and (with certain limitations) NSH proxy, and SF.

We also support MD2 TLV context headers but not yet for matching and setting, so MD2 TLVs are not part of the flow. OVS 2.8 can add MD2 context TLVs with the encap(nsh) action (classifier use case), can transparently forward MD2 headers (SFF use case) and pop an NSH header with MD2 context (final SFF use case). Support for matching and setting MD2 TLVs is FFS and can be added in a later release.

BR, Jan

^ permalink raw reply
* Re: [PATCH] DSA support for Micrel KSZ8895
From: Pavel Machek @ 2017-09-01 12:15 UTC (permalink / raw)
  To: Tristram.Ha
  Cc: Woojung.Huh, nathan.leigh.conrad, vivien.didelot, f.fainelli,
	netdev, linux-kernel, andrew
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD4111FA48@CHN-SV-EXMX02.mchp-main.com>

[-- Attachment #1: Type: text/plain, Size: 2295 bytes --]

Hi!

On Wed 2017-08-30 21:32:07, Tristram.Ha@microchip.com wrote:
> > On Mon 2017-08-28 16:09:27, Andrew Lunn wrote:
> > > > I may be confused here, but AFAICT:
> > > >
> > > > 1) Yes, it has standard layout when accessed over MDIO.
> > >
> > >
> > > Section 4.8 of the datasheet says:
> > >
> > > 	All the registers defined in this section can be also accessed
> > > 	via the SPI interface.
> > >
> > > Meaning all PHY registers can be access via the SPI interface. So you
> > > should be able to make a standard Linux MDIO bus driver which performs
> > > SPI reads.
> > 
> > As far as I can tell (and their driver confirms) -- yes, all those registers can be
> > accessed over the SPI, they are just shuffled around... hence MDIO
> > emulation code. I copied it from their code (see the copyrights) so no, I don't
> > believe there's nicer solution.
> > 
> > Best regards,
> 
> Can you hold on your developing work on KSZ8895 driver?  I am afraid your effort may be in vain.  We at Microchip are planning to release DSA drivers for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.
>

Well, thanks for heads up... but its too late to stop now. I already
have working code, without the advanced features.

I don't know how far away you are with the development. You may want
to start from my driver (but its probably too late now).

> The driver files all follow the structures of the current KSZ9477 DSA driver, and the file tag_ksz.c will be updated to handle the tail tag of different chips, which requires including the ksz_priv.h header.  That is required nevertheless to support using the offload_fwd_mark indication.
> 
> The KSZ8795 driver will be submitted after Labor Day (9/4) if
> testing reveals no problem.  The KSZ8895 driver will be submitted
> right after that.  You should have no problem using the driver right
> away.

Well, normally world can help with the testing, too. It would be nice
to see the code, as [RFC]. There's great chance that you'll have to
modify the code to adress review feedback, which is why seeing code
soon is useful.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply
* Re: [PATCH net-next 2/2] flow_dissector: Add limits for encapsulation and EH
From: Simon Horman @ 2017-09-01 12:22 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev, alex.popov, hannes
In-Reply-To: <20170831222239.21509-3-tom@quantonium.net>

On Thu, Aug 31, 2017 at 03:22:39PM -0700, Tom Herbert wrote:
> In flow dissector there are no limits to the number of nested
> encapsulations that might be dissected which makes for a nice DOS
> attack. This patch limits for dissecting nested encapsulations
> as well as for dissecting over extension headers.
> 
> Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---
>  net/core/flow_dissector.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 5110180a3e96..1bca748de27d 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -396,6 +396,35 @@ __skb_flow_dissect_ipv6(const struct sk_buff *skb,
>  	key_ip->ttl = iph->hop_limit;
>  }
>  
> +/* Maximum number of nested encapsulations that can be processed in
> + * __skb_flow_dissect
> + */
> +#define MAX_FLOW_DISSECT_ENCAPS	5
> +
> +static bool skb_flow_dissect_encap_allowed(int *num_encaps, unsigned int *flags)
> +{
> +	++*num_encaps;
> +
> +	if (*num_encaps >= MAX_FLOW_DISSECT_ENCAPS) {
> +		if (*num_encaps == MAX_FLOW_DISSECT_ENCAPS) {
> +			/* Allow one more pass but ignore disregard

It seems that 'ignore' or 'disregard' should be dropped from the text above.

> +			 * further encapsulations
> +			 */
> +			*flags |= FLOW_DISSECTOR_F_STOP_AT_ENCAP;
> +		} else {
> +			/* Max encaps reached */
> +			return  false;

There are two spaces between 'return' and 'false'.

...

^ permalink raw reply
* Re: [PATCH net-next 1/2] flow_dissector: Cleanup control flow
From: Simon Horman @ 2017-09-01 12:26 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev, alex.popov, hannes
In-Reply-To: <20170831222239.21509-2-tom@quantonium.net>

Hi Tom,

On Thu, Aug 31, 2017 at 03:22:38PM -0700, Tom Herbert wrote:
> __skb_flow_dissect is riddled with gotos that make discerning the flow,
> debugging, and extending the capability difficult. This patch
> reorganizes things so that we only perform goto's after the two main
> switch statements (no gotos within the cases now). It also eliminates
> several goto labels so that there are only two labels that can be target
> for goto.

I agree that the flow of __skb_flow_dissect() is difficult to follow
but its not obvious that this significant change in terms of loc
takes us to a better place.

Maybe it makes follow-up work easier. If so perhaps it should be motivated
along those lines.

In any case I won't stand in the way of this change but I did want to throw
my 2c worth in.

> 
> Reported-by: Alexander Popov <alex.popov@linux.com>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---
>  include/net/flow_dissector.h |   9 ++
>  net/core/flow_dissector.c    | 225 ++++++++++++++++++++++++++++---------------
>  2 files changed, 156 insertions(+), 78 deletions(-)
> 
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index e2663e900b0a..c358c3ff6acc 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -19,6 +19,15 @@ struct flow_dissector_key_control {
>  #define FLOW_DIS_FIRST_FRAG	BIT(1)
>  #define FLOW_DIS_ENCAPSULATION	BIT(2)
>  
> +enum flow_dissect_ret {
> +	FLOW_DISSECT_RET_OUT_GOOD,
> +	FLOW_DISSECT_RET_OUT_BAD,
> +	FLOW_DISSECT_RET_PROTO_AGAIN,
> +	FLOW_DISSECT_RET_IPPROTO_AGAIN,
> +	FLOW_DISSECT_RET_IPPROTO_AGAIN_EH,
> +	FLOW_DISSECT_RET_CONTINUE,
> +};

Minor nit:

My reading is that this patch does not seem to differentiate between the
handling of FLOW_DISSECT_RET_IPPROTO_AGAIN and
FLOW_DISSECT_RET_IPPROTO_AGAIN_EH.  Perhaps it would be better to add
FLOW_DISSECT_RET_IPPROTO_AGAIN_EH in the following patch where it is used.

> +
>  /**
>   * struct flow_dissector_key_basic:
>   * @thoff: Transport header offset

...

^ permalink raw reply
* Re: [PATCH net-next 1/2] flow_dissector: Cleanup control flow
From: Hannes Frederic Sowa @ 2017-09-01 12:35 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev, alex.popov
In-Reply-To: <20170831222239.21509-2-tom@quantonium.net>

Tom Herbert <tom@quantonium.net> writes:

> __skb_flow_dissect is riddled with gotos that make discerning the flow,
> debugging, and extending the capability difficult. This patch
> reorganizes things so that we only perform goto's after the two main
> switch statements (no gotos within the cases now). It also eliminates
> several goto labels so that there are only two labels that can be target
> for goto.

The problem with the

fdret = ... ;
break;

is that we now have to count curly braces to look what break does
actually break and where fdret is being processed. With the number of
context lines you send for the patch this is hard to review.

I tend to like the gotos a bit more for now.

[...]

Bye,
Hannes

^ permalink raw reply
* [PATCH net] udp: fix secpath leak
From: Paolo Abeni @ 2017-09-01 12:42 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Yossi Kuperman, Paul Moore

From: Yossi Kuperman <yossiku@mellanox.com>

After commit dce4551cb2ad ("udp: preserve head state for IP_CMSG_PASSSEC")
we preserve the secpath for the whole skb lifecycle, but we also
end up leaking a reference to it.

We must clear the head state on skb reception, if secpath is
present.

Fixes: dce4551cb2ad ("udp: preserve head state for IP_CMSG_PASSSEC")
Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/ipv4/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index a6dc48d76a29..62344804baae 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1176,7 +1176,7 @@ static void udp_set_dev_scratch(struct sk_buff *skb)
 	scratch->csum_unnecessary = !!skb_csum_unnecessary(skb);
 	scratch->is_linear = !skb_is_nonlinear(skb);
 #endif
-	if (likely(!skb->_skb_refdst))
+	if (likely(!skb->_skb_refdst && !skb_sec_path(skb)))
 		scratch->_tsize_state |= UDP_SKB_IS_STATELESS;
 }
 
-- 
2.13.5

^ permalink raw reply related
* Re: [PATCH net-next] doc: document MSG_ZEROCOPY
From: Jesper Dangaard Brouer @ 2017-09-01 12:44 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: brouer, netdev, davem, Willem de Bruijn
In-Reply-To: <20170831210013.85220-1-willemdebruijn.kernel@gmail.com>

On Thu, 31 Aug 2017 17:00:13 -0400
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> +More Info
> +---------
> +
> +Much of this document was derived from a longer paper presented at
> +netdev 2.1. For more in-depth information see that paper and talk,
> +the excellent reporting over at LWN.net or read the original code.
> +
> +  paper, slides, video
> +    https://netdevconf.org/2.1/session.html?debruijn
> +
> +  LWN article
> +    https://lwn.net/Articles/726917/
> +
> +  patchset
> +    [PATCH net-next v4 0/9] socket sendmsg MSG_ZEROCOPY
> +    https://lwn.net/Articles/730010/
> +    https://www.spinics.net/lists/netdev/msg447552.html

IMHO I think it would be better to use the type links also used in the
git log.  If you look at the kernel git log, then the "Link:" tag have
the form: http://lkml.kernel.org/r/
And you can simply append the "Message-Id:" email header.

In this case the link would be:
  http://lkml.kernel.org/r/20170803202945.70750-1-willemdebruijn.kernel@gmail.com
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply
* Re: [PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module
From: Stephen Smalley @ 2017-09-01 12:50 UTC (permalink / raw)
  To: Chenbo Feng, linux-security-module
  Cc: Chenbo Feng, SELinux, netdev, Alexei Starovoitov, lorenzo
In-Reply-To: <20170831205635.80256-2-chenbofeng.kernel@gmail.com>

On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduce 5 LSM hooks to provide finer granularity controls on eBPF
> related operations including create eBPF maps, modify and read eBPF
> maps
> content and load eBPF programs to the kernel. Hooks use the new
> security
> pointer inside the eBPF map struct to store the owner's security
> information and the different security modules can perform different
> checks based on the information stored inside the security field.
> 
> Signed-off-by: Chenbo Feng <fengc@google.com>
> ---
>  include/linux/lsm_hooks.h | 41
> +++++++++++++++++++++++++++++++++++++++++
>  include/linux/security.h  | 36 ++++++++++++++++++++++++++++++++++++
>  security/security.c       | 28 ++++++++++++++++++++++++++++
>  3 files changed, 105 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ce02f76a6188..3aaf9a08a983 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1353,6 +1353,32 @@
>   *	@inode we wish to get the security context of.
>   *	@ctx is a pointer in which to place the allocated security
> context.
>   *	@ctxlen points to the place to put the length of @ctx.
> + *
> + * Security hooks for using the eBPF maps and programs
> functionalities through
> + * eBPF syscalls.
> + *
> + * @bpf_map_create:
> + *	Check permissions prior to creating a new bpf map.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_map_modify:
> + *	Check permission prior to insert, update and delete map
> content.
> + *	@map pointer to the struct bpf_map that contains map
> information.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_map_read:
> + *	Check permission prior to read a bpf map content.
> + *	@map pointer to the struct bpf_map that contains map
> information.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_prog_load:
> + *	Check permission prior to load eBPF program.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_post_create:
> + *	Initialize the bpf object security field inside struct
> bpf_maps and
> + *	it is used for future security checks.
> + *
>   */
>  union security_list_options {
>  	int (*binder_set_context_mgr)(struct task_struct *mgr);
> @@ -1685,6 +1711,14 @@ union security_list_options {
>  				struct audit_context *actx);
>  	void (*audit_rule_free)(void *lsmrule);
>  #endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +	int (*bpf_map_create)(void);
> +	int (*bpf_map_read)(struct bpf_map *map);
> +	int (*bpf_map_modify)(struct bpf_map *map);
> +	int (*bpf_prog_load)(void);
> +	int (*bpf_post_create)(struct bpf_map *map);
> +#endif /* CONFIG_BPF_SYSCALL */
>  };
>  
>  struct security_hook_heads {
> @@ -1905,6 +1939,13 @@ struct security_hook_heads {
>  	struct list_head audit_rule_match;
>  	struct list_head audit_rule_free;
>  #endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_BPF_SYSCALL
> +	struct list_head bpf_map_create;
> +	struct list_head bpf_map_read;
> +	struct list_head bpf_map_modify;
> +	struct list_head bpf_prog_load;
> +	struct list_head bpf_post_create;
> +#endif /* CONFIG_BPF_SYSCALL */
>  } __randomize_layout;
>  
>  /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 458e24bea2d4..0656a4f74d14 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -31,6 +31,7 @@
>  #include <linux/string.h>
>  #include <linux/mm.h>
>  #include <linux/fs.h>
> +#include <linux/bpf.h>
>  
>  struct linux_binprm;
>  struct cred;
> @@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct
> dentry *dentry)
>  
>  #endif
>  
> +#ifdef CONFIG_BPF_SYSCALL
> +#ifdef CONFIG_SECURITY
> +int security_map_create(void);
> +int security_map_modify(struct bpf_map *map);
> +int security_map_read(struct bpf_map *map);
> +int security_prog_load(void);
> +int security_post_create(struct bpf_map *map);
> +#else
> +static inline int security_map_create(void)
> +{
> +	return 0;
> +}
> +
> +static inline int security_map_read(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +
> +static inline int security_map_modify(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +
> +static inline int security_prog_load(void)
> +{
> +	return 0;
> +}
> +
> +static inline int security_post_create(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_SECURITY */
> +#endif /* CONFIG_BPF_SYSCALL */

These should be named consistently with the ones in lsm_hooks.h and
should unambiguously indicate that these are hooks for bpf
objects/operations, i.e. security_bpf_map_create(),
security_bpf_map_read(), etc.

Do you need this level of granularity?

Could you coalesce the map_create() and post_map_create() hooks into
one hook and just unwind the create in that case?

Why do you label bpf maps but not bpf progs?  Should we be controlling
the ability to attach/detach a bpf prog (partly controlled by
CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
control based on who created the prog)?

Should there be a top-level security_bpf_use() hook and permission
check that limits ability to use bpf() at all?

> +
>  #ifdef CONFIG_SECURITY
>  
>  static inline char *alloc_secdata(void)
> diff --git a/security/security.c b/security/security.c
> index 55b5997e4b72..02272f93a89e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -12,6 +12,7 @@
>   *	(at your option) any later version.
>   */
>  
> +#include <linux/bpf.h>
>  #include <linux/capability.h>
>  #include <linux/dcache.h>
>  #include <linux/module.h>
> @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32
> field, u32 op, void *lsmrule,
>  				actx);
>  }
>  #endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +int security_map_create(void)
> +{
> +	return call_int_hook(bpf_map_create, 0);
> +}
> +
> +int security_map_modify(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_map_modify, 0, map);
> +}
> +
> +int security_map_read(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_map_read, 0, map);
> +}
> +
> +int security_prog_load(void)
> +{
> +	return call_int_hook(bpf_prog_load, 0);
> +}
> +
> +int security_post_create(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_post_create, 0, map);
> +}
> +#endif /* CONFIG_BPF_SYSCALL */

^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-01 13:03 UTC (permalink / raw)
  To: Vinicius Costa Gomes
  Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
	ivan.briano, jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170901012625.14838-1-vinicius.gomes@intel.com>


I happy to see this posted.  At first glance, it seems like a step in
the right direction.

On Thu, Aug 31, 2017 at 06:26:20PM -0700, Vinicius Costa Gomes wrote:
>  * Time-aware shaper (802.1Qbv):
...
>    S 0x01 300
>    S 0x03 500
> 
>    This means that there are two intervals, the first will have the gate
>    for traffic class 0 open for 300 nanoseconds, the second will have
>    both traffic classes open for 500 nanoseconds.

The i210 doesn't support this in HW, or does it?

>  * Frame Preemption (802.1Qbu):
> 
>    To control even further the latency, it may prove useful to signal which
>    traffic classes are marked as preemptable. For that, 'taprio' provides the
>    preemption command so you set each traffic class as preemptable or not:
> 
>    $ tc qdisc (...) \
>         preemption 0 1 1 1

Neither can the i210 preempt frames, or what am I missing?

The timing of this RFC is good, as I am just finishing up an RFC that
implements time-based transmit using the i210.  I'll try and get that
out ASAP.

Thanks,
Richard

^ permalink raw reply
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Mike Galbraith @ 2017-09-01 13:09 UTC (permalink / raw)
  To: Kees Cook
  Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
	Reshetova, Elena, Network Development
In-Reply-To: <1504249070.17604.20.camel@gmx.de>

On Fri, 2017-09-01 at 08:57 +0200, Mike Galbraith wrote:
> On Thu, 2017-08-31 at 11:45 -0700, Kees Cook wrote:
> > On Thu, Aug 31, 2017 at 10:19 AM, Mike Galbraith <efault@gmx.de> wrote:
> > > On Thu, 2017-08-31 at 10:00 -0700, Kees Cook wrote:
> > >>
> > >> Oh! So it's gcc-version sensitive? That's alarming. Is this mapping correct:
> > >>
> > >> 4.8.5: WARN, eventual kernel hang
> > >> 6.3.1, 7.0.1: WARN, but continues working
> > >
> > > Yeah, that's correct.  I find that troubling, simply because this gcc
> > > version has been through one hell of a lot of kernels with me.  Yeah, I
> > > know, that doesn't exempt it from having bugs, but color me suspicious.
> > 
> > I still can't hit this with a 4.8.5 build. :(
> > 
> > With _RATELIMIT removed, this should, in theory, report whatever goes
> > negative first...
> 
> I applied the other patch you posted, and built with gcc-6.3.1 to
> remove the gcc-4.8.5 aspect.  Look below the resulting splat.

Grr, that one has a in6_dev_getx() line missing for the first
increment, where things go pear shaped.

With that added, looking at counter both before, and after incl, with a
trace_printk() in the exception handler showing it doing its saturate
thing, irqs disabled across the whole damn refcount_inc(), and even
booting box nr_cpus=1 for extra credit...

HTH can that first refcount_inc() get there?

# tracer: nop
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
         systemd-1     [000] d..1     1.937284: in6_dev_getx: PRE refs.counter:3
         systemd-1     [000] d..1     1.937295: ex_handler_refcount: *(int *)regs->cx = -1073741824
         systemd-1     [000] d..1     1.937296: in6_dev_getx: POST refs.counter:-1073741824
         systemd-1     [000] d..1     1.937296: in6_dev_getx: PRE refs.counter:-1073741824
         systemd-1     [000] d..1     1.937297: ex_handler_refcount: *(int *)regs->cx = -1073741824
         systemd-1     [000] d..1     1.937297: in6_dev_getx: POST refs.counter:-1073741824
         systemd-1     [000] d..1     1.937297: in6_dev_getx: PRE refs.counter:-1073741824
         systemd-1     [000] d..1     1.937298: ex_handler_refcount: *(int *)regs->cx = -1073741824
         systemd-1     [000] d..1     1.937299: in6_dev_getx: POST refs.counter:-1073741824

---
 arch/x86/include/asm/refcount.h |   14 ++++++++++++++
 arch/x86/mm/extable.c           |    1 +
 include/net/addrconf.h          |   12 ++++++++++++
 net/ipv6/route.c                |    6 +++---
 4 files changed, 30 insertions(+), 3 deletions(-)

--- a/arch/x86/include/asm/refcount.h
+++ b/arch/x86/include/asm/refcount.h
@@ -55,6 +55,20 @@ static __always_inline void refcount_inc
 		: : "cc", "cx");
 }
 
+static __always_inline void refcount_inc_x(refcount_t *r)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	trace_printk("PRE refs.counter:%d\n", r->refs.counter);
+	asm volatile(LOCK_PREFIX "incl %0\n\t"
+		REFCOUNT_CHECK_LT_ZERO
+		: [counter] "+m" (r->refs.counter)
+		: : "cc", "cx");
+	trace_printk("POST refs.counter:%d\n", r->refs.counter);
+	local_irq_restore(flags);
+}
+
 static __always_inline void refcount_dec(refcount_t *r)
 {
 	asm volatile(LOCK_PREFIX "decl %0\n\t"
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -45,6 +45,7 @@ bool ex_handler_refcount(const struct ex
 {
 	/* First unconditionally saturate the refcount. */
 	*(int *)regs->cx = INT_MIN / 2;
+	trace_printk("*(int *)regs->cx = %d\n", *(int *)regs->cx);
 
 	/*
 	 * Strictly speaking, this reports the fixup destination, not
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -321,6 +321,18 @@ static inline struct inet6_dev *in6_dev_
 	return idev;
 }
 
+static inline struct inet6_dev *in6_dev_getx(const struct net_device *dev)
+{
+	struct inet6_dev *idev;
+
+	rcu_read_lock();
+	idev = rcu_dereference(dev->ip6_ptr);
+	if (idev)
+		refcount_inc_x(&idev->refcnt);
+	rcu_read_unlock();
+	return idev;
+}
+
 static inline struct neigh_parms *__in6_dev_nd_parms_get_rcu(const struct net_device *dev)
 {
 	struct inet6_dev *idev = __in6_dev_get(dev);
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4041,12 +4041,12 @@ void __init ip6_route_init_special_entri
 	 * the loopback reference in rt6_info will not be taken, do it
 	 * manually for init_net */
 	init_net.ipv6.ip6_null_entry->dst.dev = init_net.loopback_dev;
-	init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
+	init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_getx(init_net.loopback_dev);
   #ifdef CONFIG_IPV6_MULTIPLE_TABLES
 	init_net.ipv6.ip6_prohibit_entry->dst.dev = init_net.loopback_dev;
-	init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
+	init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_getx(init_net.loopback_dev);
 	init_net.ipv6.ip6_blk_hole_entry->dst.dev = init_net.loopback_dev;
-	init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
+	init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_getx(init_net.loopback_dev);
   #endif
 }
 

^ permalink raw reply
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Andrew Lunn @ 2017-09-01 13:29 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: jiri, jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <7d738ef5-c312-e0b3-3605-1f31fa7dc019@gmail.com>

> I suppose that you could somehow use TC to influence how the traffic
> from host to CPU works, but without a "CPU" port representor the
> question is how do we get that done? If we used "eth0" we need to
> callback into the switch driver for programming..

We need to compare how the different switches work with respect to
QoS. Marvell switches do a lot of the classification on the ingress
port where it determines what queue the frame should be placed in on
the egress port. The egress port then schedules its queues.

This does not map to TC too well.

> Regarding the last patch in this series, what I would ideally to replace
> it with is something along the lines of:
> 
> tc bind dev sw0p0 queue 0 dev eth0 queue 16

Why do you need this? sw0p0 has 8 queues? So i assume you use TC on
sw0p0 to place frames into these queues? The queue then gets passed
transparently down through the conduit interface and then used by the
tagger. I don't see why you need eth0 here? We try our best to avoid
eth0 wherever possible, it causes confusion. So i would prefer not to
have to use eth0 with TC commands.

     Andrew

^ permalink raw reply
* Re: [PATCH net-next 2/2] flow_dissector: Add limits for encapsulation and EH
From: Hannes Frederic Sowa @ 2017-09-01 13:32 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev, alex.popov
In-Reply-To: <20170831222239.21509-3-tom@quantonium.net>

Tom Herbert <tom@quantonium.net> writes:

> In flow dissector there are no limits to the number of nested
> encapsulations that might be dissected which makes for a nice DOS
> attack. This patch limits for dissecting nested encapsulations
> as well as for dissecting over extension headers.

I was actually more referring to your patch, because the flow dissector
right now is not stack recursive. Your changes would make it doing
recursion on the stack. But it seems something along the lines is anyway
needed. See below.

> Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---
>  net/core/flow_dissector.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 5110180a3e96..1bca748de27d 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -396,6 +396,35 @@ __skb_flow_dissect_ipv6(const struct sk_buff *skb,
>  	key_ip->ttl = iph->hop_limit;
>  }
>  
> +/* Maximum number of nested encapsulations that can be processed in
> + * __skb_flow_dissect
> + */
> +#define MAX_FLOW_DISSECT_ENCAPS	5

I think you can exactly parse one encapsulation layer safely. This is
because you can only keep state on one encapsulation layer protocol
right now.

For example this scenario:

I would like to circumvent tc flower rules that filter based on vlan. I
simply construct a packet looking like:

Ethernet|Vlan|IP|GRE|Ethernet|Vlan|

because we don't recurse in the flow keys either, the second vlan header
would overwrite the information of the first one.

> +
> +static bool skb_flow_dissect_encap_allowed(int *num_encaps, unsigned int *flags)
> +{
> +	++*num_encaps;
> +
> +	if (*num_encaps >= MAX_FLOW_DISSECT_ENCAPS) {
> +		if (*num_encaps == MAX_FLOW_DISSECT_ENCAPS) {
> +			/* Allow one more pass but ignore disregard
> +			 * further encapsulations
> +			 */
> +			*flags |= FLOW_DISSECTOR_F_STOP_AT_ENCAP;
> +		} else {
> +			/* Max encaps reached */
> +			return  false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
> +/* Maximum number of extension headers can be processed in __skb_flow_dissect
> + * per IPv6 packet
> + */
> +#define MAX_FLOW_DISSECT_EH	5

I would at least allow each extension header once, DEST_OPS twice, thus
let's say 12? It is safe in this version because it does not consume
stack space anyway and doesn't update internal state.

> +
>  /**
>   * __skb_flow_dissect - extract the flow_keys struct and return it
>   * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
> @@ -426,6 +455,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>  	struct flow_dissector_key_tags *key_tags;
>  	struct flow_dissector_key_vlan *key_vlan;
>  	enum flow_dissect_ret fdret;
> +	int num_eh, num_encaps = 0;
>  	bool skip_vlan = false;
>  	u8 ip_proto = 0;
>  	bool ret;
> @@ -714,7 +744,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>  	case FLOW_DISSECT_RET_OUT_GOOD:
>  		goto out_good;
>  	case FLOW_DISSECT_RET_PROTO_AGAIN:
> -		goto proto_again;
> +		if (skb_flow_dissect_encap_allowed(&num_encaps, &flags))
> +			goto proto_again;

I think you should get the check to the proto_again label. In case you
loop to often you can `goto out_good'.

[...]

Bye,
Hannes

^ 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