Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net V2] net/mlx4: Fix the check in attaching steering rules
From: Tariq Toukan @ 2017-05-25 14:23 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1495718470-10215-1-git-send-email-tariqt@mellanox.com>


On 25/05/2017 4:21 PM, Tariq Toukan wrote:
> From: Talat Batheesh <talatb@mellanox.com>
>
> Our previous patch (cited below) introduced a regression
> for RAW Eth QPs.
>
> Fix it by checking if the QP number provided by user-space
> exists, hence allowing steering rules to be added for valid
> QPs only.
>
> Fixes: 89c557687a32 ("net/mlx4_en: Avoid adding steering rules with ...")
> Reported-by: Or Gerlitz <gerlitz.or@gmail.com>
> Signed-off-by: Talat Batheesh <talatb@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Acked-by: Or Gerlitz <ogerlitz@mellanox.com>
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> v2:
> * Removed unnecessary EXPORT_SYMBOL_GPL.

My bad, this doesn't work.
Please ignore patch.

>
>   drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |  3 +--
>   drivers/net/ethernet/mellanox/mlx4/qp.c         | 13 +++++++++++++
>   include/linux/mlx4/qp.h                         |  1 +
>   3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> index ae5fdc2df654..00a7cd3dcc2e 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> @@ -1562,8 +1562,7 @@ static int mlx4_en_flow_replace(struct net_device *dev,
>   		qpn = priv->drop_qp.qpn;
>   	else if (cmd->fs.ring_cookie & EN_ETHTOOL_QP_ATTACH) {
>   		qpn = cmd->fs.ring_cookie & (EN_ETHTOOL_QP_ATTACH - 1);
> -		if (qpn < priv->rss_map.base_qpn ||
> -		    qpn >= priv->rss_map.base_qpn + priv->rx_ring_num) {
> +		if (!mlx4_qp_lookup(priv->mdev->dev, qpn)) {
>   			en_warn(priv, "rxnfc: QP (0x%x) doesn't exist\n", qpn);
>   			return -EINVAL;
>   		}
> diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c b/drivers/net/ethernet/mellanox/mlx4/qp.c
> index 2d6abd4662b1..ad92d2311478 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/qp.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
> @@ -384,6 +384,19 @@ static void mlx4_qp_free_icm(struct mlx4_dev *dev, int qpn)
>   		__mlx4_qp_free_icm(dev, qpn);
>   }
>   
> +struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn)
> +{
> +	struct mlx4_qp_table *qp_table = &mlx4_priv(dev)->qp_table;
> +	struct mlx4_qp *qp;
> +
> +	spin_lock(&qp_table->lock);
> +
> +	qp = __mlx4_qp_lookup(dev, qpn);
> +
> +	spin_unlock(&qp_table->lock);
> +	return qp;
> +}
> +
>   int mlx4_qp_alloc(struct mlx4_dev *dev, int qpn, struct mlx4_qp *qp, gfp_t gfp)
>   {
>   	struct mlx4_priv *priv = mlx4_priv(dev);
> diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
> index b4ee8f62ce8d..8e2828d48d7f 100644
> --- a/include/linux/mlx4/qp.h
> +++ b/include/linux/mlx4/qp.h
> @@ -470,6 +470,7 @@ struct mlx4_update_qp_params {
>   	u16	rate_val;
>   };
>   
> +struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn);
>   int mlx4_update_qp(struct mlx4_dev *dev, u32 qpn,
>   		   enum mlx4_update_qp_attr attr,
>   		   struct mlx4_update_qp_params *params);

^ permalink raw reply

* Re: [PATCH][net-next] qtnfmac: remove duplicated assignment to mac
From: Sergey Matyukevich @ 2017-05-25 13:56 UTC (permalink / raw)
  To: Colin King
  Cc: Avinash Patil, Sergey Matyukevich, Igor Mitsyanko, Kalle Valo,
	Huizhao Wang, Kamlesh Rath, linux-wireless, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20170525134503.12106-1-colin.king@canonical.com>

> mac is being assigned twice, remove redundant 2nd assignment.
> 
> Detected by CoverityScan, CID#1437554 ("Incorrect expression")
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
> index fc0ce2c09097..e3c090008125 100644
> --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
> +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
> @@ -922,7 +922,7 @@ void qtnf_netdev_updown(struct net_device *ndev, bool up)
>  void qtnf_virtual_intf_cleanup(struct net_device *ndev)
>  {
>         struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
> -       struct qtnf_wmac *mac = mac = wiphy_priv(vif->wdev.wiphy);
> +       struct qtnf_wmac *mac = wiphy_priv(vif->wdev.wiphy);

Thanks !

Acked-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

^ permalink raw reply

* [PATCH net-next v2] ibmvnic: Enable TSO support
From: Thomas Falcon @ 2017-05-25 13:55 UTC (permalink / raw)
  To: netdev; +Cc: Thomas Falcon, Nathan Fontenot, John Allen

Enable TSO in the ibmvnic driver. Scatter-gather is also enabled,
though there currently is no support for scatter-gather in
vNIC-compatible hardware, resulting in forced linearization
of all fragmented SKB's. Though not ideal, given the throughput
improvement gained by enabling TSO, it has been decided
that this is an acceptable tradeoff.

The feature is also enabled by a module parameter.
This parameter is necessary because TSO can not easily be
enabled or disabled in firmware without reinitializing the driver.

CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
CC: John Allen <jallen@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
v2: Fix kbuild robot warning from module parameter init
---
 drivers/net/ethernet/ibm/ibmvnic.c | 39 +++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index abe2b6e..29cb2c4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -81,6 +81,11 @@
 static const char ibmvnic_driver_name[] = "ibmvnic";
 static const char ibmvnic_driver_string[] = "IBM System i/p Virtual NIC Driver";
 
+static bool large_send_offload;
+module_param(large_send_offload, bool, 0644);
+MODULE_PARM_DESC(large_send_offload,
+		 "Determines whether large send offload is enabled");
+
 MODULE_AUTHOR("Santiago Leon <santi_leon@yahoo.com>");
 MODULE_DESCRIPTION("IBM System i/p Virtual NIC Driver");
 MODULE_LICENSE("GPL");
@@ -1025,6 +1030,17 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		goto out;
 	}
 
+	/* All scatter-gather SKB's will be linearized for the time being, but
+	 * scatter-gather is only enabled if the user wishes to use TSO.
+	 */
+	if (skb_shinfo(skb)->nr_frags && __skb_linearize(skb)) {
+		dev_kfree_skb_any(skb);
+		tx_send_failed++;
+		tx_dropped++;
+		ret = NETDEV_TX_OK;
+		goto out;
+	}
+
 	tx_pool = &adapter->tx_pool[queue_num];
 	tx_scrq = adapter->tx_scrq[queue_num];
 	txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
@@ -1082,6 +1098,11 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		tx_crq.v1.flags1 |= IBMVNIC_TX_CHKSUM_OFFLOAD;
 		hdrs += 2;
 	}
+	if (skb_is_gso(skb)) {
+		tx_crq.v1.flags1 |= IBMVNIC_TX_LSO;
+		tx_crq.v1.mss = cpu_to_be16(skb_shinfo(skb)->gso_size);
+		hdrs += 2;
+	}
 	/* determine if l2/3/4 headers are sent to firmware */
 	if ((*hdrs >> 7) & 1 &&
 	    (skb->protocol == htons(ETH_P_IP) ||
@@ -2629,10 +2650,10 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
 	adapter->ip_offload_ctrl.udp_ipv4_chksum = buf->udp_ipv4_chksum;
 	adapter->ip_offload_ctrl.tcp_ipv6_chksum = buf->tcp_ipv6_chksum;
 	adapter->ip_offload_ctrl.udp_ipv6_chksum = buf->udp_ipv6_chksum;
+	adapter->ip_offload_ctrl.large_tx_ipv4 = buf->large_tx_ipv4;
+	adapter->ip_offload_ctrl.large_tx_ipv6 = buf->large_tx_ipv6;
 
-	/* large_tx/rx disabled for now, additional features needed */
-	adapter->ip_offload_ctrl.large_tx_ipv4 = 0;
-	adapter->ip_offload_ctrl.large_tx_ipv6 = 0;
+	/* large_rx disabled for now, additional features needed */
 	adapter->ip_offload_ctrl.large_rx_ipv4 = 0;
 	adapter->ip_offload_ctrl.large_rx_ipv6 = 0;
 
@@ -2648,6 +2669,18 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
 	    (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)))
 		adapter->netdev->features |= NETIF_F_RXCSUM;
 
+	if (large_send_offload) {
+		/* Scatter-gather is not currently supported by firmware.
+		 * It will only be enabled to support TSO operations.
+		 */
+		adapter->netdev->features = NETIF_F_SG;
+
+		if (buf->large_tx_ipv4)
+			adapter->netdev->features |= NETIF_F_TSO;
+		if (buf->large_tx_ipv6)
+			adapter->netdev->features |= NETIF_F_TSO6;
+	}
+
 	memset(&crq, 0, sizeof(crq));
 	crq.control_ip_offload.first = IBMVNIC_CRQ_CMD;
 	crq.control_ip_offload.cmd = CONTROL_IP_OFFLOAD;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH][net-next] qtnfmac: remove duplicated assignment to mac
From: Colin King @ 2017-05-25 13:45 UTC (permalink / raw)
  To: Avinash Patil, Sergey Matyukevich, Kalle Valo, Huizhao Wang,
	Kamlesh Rath, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

mac is being assigned twice, remove redundant 2nd assignment.

Detected by CoverityScan, CID#1437554 ("Incorrect expression")

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index fc0ce2c09097..e3c090008125 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -922,7 +922,7 @@ void qtnf_netdev_updown(struct net_device *ndev, bool up)
 void qtnf_virtual_intf_cleanup(struct net_device *ndev)
 {
 	struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
-	struct qtnf_wmac *mac = mac = wiphy_priv(vif->wdev.wiphy);
+	struct qtnf_wmac *mac = wiphy_priv(vif->wdev.wiphy);
 
 	if (vif->wdev.iftype == NL80211_IFTYPE_STATION) {
 		switch (vif->sta_state) {
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Ding Tianhong @ 2017-05-25 13:35 UTC (permalink / raw)
  To: Casey Leedom, Alexander Duyck
  Cc: Mark Rutland, Gabriele Paoloni, Asit K Mallick, Catalin Marinas,
	Will Deacon, LinuxArm, Raj, Ashok, Bjorn Helgaas, Ganesh GR,
	Jeff Kirsher, Bob Shaw, Patrick J Cramer, Arjun V.,
	Michael Werner, linux-arm-kernel@lists.infradead.org, Amir Ancel,
	Netdev, David Laight, Suravee Suthikulpanit, Robin Murphy,
	David Miller, h
In-Reply-To: <MWHPR12MB1600AC957C7E3B0DB4F06D6CC8EE0@MWHPR12MB1600.namprd12.prod.outlook.com>


On 2017/5/9 8:48, Casey Leedom wrote:
> 
> | From: Alexander Duyck <alexander.duyck@gmail.com>
> | Date: Saturday, May 6, 2017 11:07 AM
> | 
> | | From: Ding Tianhong <dingtianhong@huawei.com>
> | | Date: Fri, May 5, 2017 at 8:08 PM
> | | 
> | | According the suggestion, I could only think of this code:
> | | ..
> | 
> | This is a bit simplistic but it is a start.
> 
>   Yes, something tells me that this is going to be more complicated than any
> of us like ...
> 
> | The other bit I was getting at is that we need to update the core PCIe
> | code so that when we configure devices and the root complex reports no
> | support for relaxed ordering it should be clearing the relaxed
> | ordering bits in the PCIe configuration registers on the upstream
> | facing devices.
> 
>   Of course, this can be written to by the Driver at any time ... and is in
> the case of the cxgb4 Driver ...
> 
>   After a lot of rummaging around, it does look like KVM prohibits writes to
> the PCIe Capability Device Control register in drivers/xen/xen-pciback/
> conf_space_capability.c and conf_space.c simply because writes aren't
> allowed unless "permissive" is set.  So it ~looks~ like a driver running in
> a Virtual Machine can't turn Enable Relaxed Ordering back on ...
> 
> | The last bit we need in all this is a way to allow for setups where
> | peer-to-peer wants to perform relaxed ordering but for writes to the
> | host we have to not use relaxed ordering. For that we need to enable a
> | special case and that isn't handled right now in any of the solutions
> | we have coded up so far.
> 
>   Yes, we do need this.
> 
> 
> | From: Alexander Duyck <alexander.duyck@gmail.com>
> | Date: Saturday, May 8, 2017 08:22 AM
> |
> | The problem is we need to have something that can be communicated
> | through a VM. Your change doesn't work in that regard. That was why I
> | suggested just updating the code so that we when we initialized PCIe
> | devices what we do is either set or clear the relaxed ordering bit in
> | the PCIe device control register. That way when we direct assign an
> | interface it could know just based on the bits int the PCIe
> | configuration if it could use relaxed ordering or not.
> | 
> | At that point the driver code itself becomes very simple since you
> | could just enable the relaxed ordering by default in the igb/ixgbe
> | driver and if the bit is set or cleared in the PCIe configuration then
> | we are either sending with relaxed ordering requests or not and don't
> | have to try and locate the root complex.
> | 
> | So from the sound of it Casey has a special use case where he doesn't
> | want to send relaxed ordering frames to the root complex, but instead
> | would like to send them to another PCIe device. To do that he needs to
> | have a way to enable the relaxed ordering bit in the PCIe
> | configuration but then not send any to the root complex. Odds are that
> | is something he might be able to just implement in the driver, but is
> | something that may become a more general case in the future. I don't
> | see our change here impacting it as long as we keep the solution
> | generic and mostly confined to when we instantiate the devices as the
> | driver could likely make the decision to change the behavior later.
> 
>   It's not just me.  Intel has said that while RO directed at the Root
> Complex Host Coherent Memory has a performance bug (not Data Corruption),
> it's a performance win for Peer-to-Peer writes to MMIO Space.  (I'll be very
> interested in hearing what the bug is if we get that much detail.  The very
> same TLPs directed to the Root Complex Port without Relaxed Ordering set get
> good performance.  So this is essentially a bug in the hardware that was
> ~trying~ to implement a performance win.)
> 
>   Meanwhile, I currently only know of a single PCIe End Point which causes
> catastrophic results: the AMD A1100 ARM SoC ("SEATTLE").  And it's not even
> clear that product is even alive anymore since I haven't been able to get
> any responses from them for several months.
> 
>   What I'm saying is: let's try to architect a solution which doesn't throw
> the baby out with the bath water ...
> 
>   I think that if a Device's Root Complex Port has problems with Relaxed
> Ordering, it ~probably~ makes sense to turn off the PCIe Capability Device
> Control[Enable Relaxed Ordering] when we assign a device to a Virtual
> Machine since the Device Driver can no longer query the Relaxed Ordering
> Support of the Root Complex Port.  The only down side of this would be if we
> assigned two Peers to a VM in an application which wanted to do Peer-to-Peer
> transfers.  But that seems like a hard application to support in any case
> since the PCI Bus:Slot.Function IDs for assigned Devices within a VM don't
> match the actual values.
> 
>   For Devices running in the base OS/Hypervisor, their Drivers can query the
> Relaxed Ordering Support for the Root Complex Port or a Peer Device.  So a
> simple flag within the (struct pci_dev *)->dev_flags would serve for that
> along with a per-Architecture/Platform mechanism for setting it ...
> 
> Casey
> 

I have take a time to talk to our kvm team about how to distinguish the relaxed
ordering in the VM for some vf just like 82599-vf, the probe routine looks like
could work like this:
1) QEMU could emulate the platform by the Vender ID and device ID which could be
   read from the host.
2) The QEMU could create a virtual PCIe dev complex and recognize the PCIe bus address which
   come and detach from the host to the guest.
3) the PCI quirk could enable the Relaxed Ordering by the Vendor ID and Device ID.
4) The VF drivers could read the flag and set to the hw.

So I think we could set the PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED for some special platform
and don't enable by default, if I miss something, please not hesitate to enlighten me :)

--------------------------------------------------------------
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 085fb78..74bcc25 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4664,3 +4664,22 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
+
+/*
+ * Some devices have problems with Transaction Layer Packets with the Relaxed
+ * Ordering Attribute set, so we should disable Relaxed Ordering by default
+ * and only enable it when some devices has mark themselves and other
+ * Device Drivers should check before sending TLPs with RO set.
+ */
+static void quirk_relaxedordering_enable(struct pci_dev *dev)
+{
+ dev->dev_flags &= ~PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED;
+}
+
+/*
+ * Hisilicon Root Complex could support relaxed ordering which can
+ * improve performance with Upstream Transaction Layer Packets with
+ * Relaxed Ordering set.
+ */
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_HUAWEI, 0x1610, PCI_CLASS_NOT_DEFINED, 8,
+                       quirk_relaxedordering_enable);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 33c2b0b..f7d8d6f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -183,6 +183,8 @@ enum pci_dev_flags {
        PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT = (__force pci_dev_flags_t) (1 << 9),
        /* Do not use FLR even if device advertises PCI_AF_CAP */
        PCI_DEV_FLAGS_NO_FLR_RESET = (__force pci_dev_flags_t) (1 << 10),
+ /* Use Relaxed Ordering for TLPs directed at this device */
+ PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED = (__force pci_dev_flags_t) (1 << 11),
 };

 enum pci_irq_reroute_variant {
@@ -2203,6 +2205,20 @@ static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
        return false;
 }

+/**
+ * pci_is_dev_relaxed_ordering_enabled - whether device could support Relaxed
+ * Ordering for TLPs directed.
+ * @pdev: PCI device to check
+ *
+ * This function could return the value indicates that whether Relaxed Ordering
+ * Attribute could be used on Transaction Layer Packets destined for the PCIe
+ * End Node.
+ */
+static inline boot pci_is_dev_relaxed_ordering_enabled(struct pci_dev *pdev)
+{
+ return (pdev->dev_flags & PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED) ==
+        PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED;
 /* provide the legacy pci_dma_* API */
 #include <linux/pci-dma-compat.h>

Thanks
Ding

> .
> 

^ permalink raw reply related

* [PATCH net-next 2/4] net/sched: cls_flower: add support for matching on ip tos and ttl
From: Or Gerlitz @ 2017-05-25 13:24 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Saeed Mahameed, Roi Dayan, Paul Blakey, Or Gerlitz
In-Reply-To: <1495718679-20693-1-git-send-email-ogerlitz@mellanox.com>

Benefit from the support of ip header fields dissection and
allow users to set rules matching on ipv4 tos and ttl or
ipv6 traffic-class and hoplimit.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/uapi/linux/pkt_cls.h |  5 +++++
 net/sched/cls_flower.c       | 39 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index c6e8cf5..edf43dd 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -454,6 +454,11 @@ enum {
 	TCA_FLOWER_KEY_TCP_FLAGS,	/* be16 */
 	TCA_FLOWER_KEY_TCP_FLAGS_MASK,	/* be16 */
 
+	TCA_FLOWER_KEY_IP_TOS,		/* u8 */
+	TCA_FLOWER_KEY_IP_TOS_MASK,	/* u8 */
+	TCA_FLOWER_KEY_IP_TTL,		/* u8 */
+	TCA_FLOWER_KEY_IP_TTL_MASK,	/* u8 */
+
 	__TCA_FLOWER_MAX,
 };
 
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index fb74a47..33feaee 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -50,6 +50,7 @@ struct fl_flow_key {
 	struct flow_dissector_key_ports enc_tp;
 	struct flow_dissector_key_mpls mpls;
 	struct flow_dissector_key_tcp tcp;
+	struct flow_dissector_key_ip ip;
 } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
 
 struct fl_flow_mask_range {
@@ -427,6 +428,10 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
 	[TCA_FLOWER_KEY_MPLS_LABEL]	= { .type = NLA_U32 },
 	[TCA_FLOWER_KEY_TCP_FLAGS]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_TCP_FLAGS_MASK]	= { .type = NLA_U16 },
+	[TCA_FLOWER_KEY_IP_TOS]		= { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_IP_TOS_MASK]	= { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_IP_TTL]		= { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_IP_TTL_MASK]	= { .type = NLA_U8 },
 };
 
 static void fl_set_key_val(struct nlattr **tb,
@@ -528,6 +533,19 @@ static int fl_set_key_flags(struct nlattr **tb,
 	return 0;
 }
 
+static void fl_set_key_ip(struct nlattr **tb,
+			  struct flow_dissector_key_ip *key,
+			  struct flow_dissector_key_ip *mask)
+{
+		fl_set_key_val(tb, &key->tos, TCA_FLOWER_KEY_IP_TOS,
+			       &mask->tos, TCA_FLOWER_KEY_IP_TOS_MASK,
+			       sizeof(key->tos));
+
+		fl_set_key_val(tb, &key->ttl, TCA_FLOWER_KEY_IP_TTL,
+			       &mask->ttl, TCA_FLOWER_KEY_IP_TTL_MASK,
+			       sizeof(key->ttl));
+}
+
 static int fl_set_key(struct net *net, struct nlattr **tb,
 		      struct fl_flow_key *key, struct fl_flow_key *mask)
 {
@@ -570,6 +588,7 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 		fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
 			       &mask->basic.ip_proto, TCA_FLOWER_UNSPEC,
 			       sizeof(key->basic.ip_proto));
+		fl_set_key_ip(tb, &key->ip, &mask->ip);
 	}
 
 	if (tb[TCA_FLOWER_KEY_IPV4_SRC] || tb[TCA_FLOWER_KEY_IPV4_DST]) {
@@ -773,6 +792,8 @@ static void fl_init_dissector(struct cls_fl_head *head,
 	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
 			     FLOW_DISSECTOR_KEY_PORTS, tp);
 	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+			     FLOW_DISSECTOR_KEY_IP, ip);
+	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
 			     FLOW_DISSECTOR_KEY_TCP, tcp);
 	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ICMP, icmp);
@@ -1082,6 +1103,19 @@ static int fl_dump_key_mpls(struct sk_buff *skb,
 	return 0;
 }
 
+static int fl_dump_key_ip(struct sk_buff *skb,
+			  struct flow_dissector_key_ip *key,
+			  struct flow_dissector_key_ip *mask)
+{
+	if (fl_dump_key_val(skb, &key->tos, TCA_FLOWER_KEY_IP_TOS, &mask->tos,
+			    TCA_FLOWER_KEY_IP_TOS_MASK, sizeof(key->tos)) ||
+	    fl_dump_key_val(skb, &key->ttl, TCA_FLOWER_KEY_IP_TTL, &mask->ttl,
+			    TCA_FLOWER_KEY_IP_TTL_MASK, sizeof(key->ttl)))
+		return -1;
+
+	return 0;
+}
+
 static int fl_dump_key_vlan(struct sk_buff *skb,
 			    struct flow_dissector_key_vlan *vlan_key,
 			    struct flow_dissector_key_vlan *vlan_mask)
@@ -1195,9 +1229,10 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 
 	if ((key->basic.n_proto == htons(ETH_P_IP) ||
 	     key->basic.n_proto == htons(ETH_P_IPV6)) &&
-	    fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
+	    (fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
 			    &mask->basic.ip_proto, TCA_FLOWER_UNSPEC,
-			    sizeof(key->basic.ip_proto)))
+			    sizeof(key->basic.ip_proto)) ||
+	    fl_dump_key_ip(skb, &key->ip, &mask->ip)))
 		goto nla_put_failure;
 
 	if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
-- 
2.3.7

^ permalink raw reply related

* [PATCH net-next 0/4] add support for dissection and matching on ip tos and ttl
From: Or Gerlitz @ 2017-05-25 13:24 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Saeed Mahameed, Roi Dayan, Paul Blakey, Or Gerlitz

Hi Dave, 

The 1st two patches enable matching/classifying on ip tos and ttl by 
the flow dissector and flower. The other two patches offload matching 
on tcp flags and ip tos in mlx5. 

The mlx5 patches touch single file/function and not interfere with
other inflight mlx5 submissions.

Or.


Or Gerlitz (4):
  net/flow_dissector: add support for dissection of misc ip header fields
  net/sched: cls_flower: add support for matching on ip tos and ttl
  net/mlx5e: Offload TC matching on tcp flags
  net/mlx5e: Offload TC matching on ip tos / traffic-class

 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 46 ++++++++++++++++++++++++-
 include/net/flow_dissector.h                    | 11 ++++++
 include/uapi/linux/pkt_cls.h                    |  5 +++
 net/core/flow_dissector.c                       | 40 +++++++++++++++++++++
 net/sched/cls_flower.c                          | 39 +++++++++++++++++++--
 5 files changed, 138 insertions(+), 3 deletions(-)

-- 
2.3.7

^ permalink raw reply

* [PATCH net-next 3/4] net/mlx5e: Offload TC matching on tcp flags
From: Or Gerlitz @ 2017-05-25 13:24 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Saeed Mahameed, Roi Dayan, Paul Blakey, Or Gerlitz
In-Reply-To: <1495718679-20693-1-git-send-email-ogerlitz@mellanox.com>

Enable offloading of TC matching on tcp flags.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index a72ecbc..8e856cd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -580,7 +580,8 @@ static int __parse_cls_flower(struct mlx5e_priv *priv,
 	      BIT(FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS) |
 	      BIT(FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS) |
 	      BIT(FLOW_DISSECTOR_KEY_ENC_PORTS)	|
-	      BIT(FLOW_DISSECTOR_KEY_ENC_CONTROL))) {
+	      BIT(FLOW_DISSECTOR_KEY_ENC_CONTROL) |
+	      BIT(FLOW_DISSECTOR_KEY_TCP))) {
 		netdev_warn(priv->netdev, "Unsupported key used: 0x%x\n",
 			    f->dissector->used_keys);
 		return -EOPNOTSUPP;
@@ -807,6 +808,25 @@ static int __parse_cls_flower(struct mlx5e_priv *priv,
 			*min_inline = MLX5_INLINE_MODE_TCP_UDP;
 	}
 
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_TCP)) {
+		struct flow_dissector_key_tcp *key =
+			skb_flow_dissector_target(f->dissector,
+						  FLOW_DISSECTOR_KEY_TCP,
+						  f->key);
+		struct flow_dissector_key_tcp *mask =
+			skb_flow_dissector_target(f->dissector,
+						  FLOW_DISSECTOR_KEY_TCP,
+						  f->mask);
+
+		MLX5_SET(fte_match_set_lyr_2_4, headers_c, tcp_flags,
+			 ntohs(mask->flags));
+		MLX5_SET(fte_match_set_lyr_2_4, headers_v, tcp_flags,
+			 ntohs(key->flags));
+
+		if (mask->flags)
+			*min_inline = MLX5_INLINE_MODE_TCP_UDP;
+	}
+
 	return 0;
 }
 
-- 
2.3.7

^ permalink raw reply related

* [PATCH net-next 1/4] net/flow_dissector: add support for dissection of misc ip header fields
From: Or Gerlitz @ 2017-05-25 13:24 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Saeed Mahameed, Roi Dayan, Paul Blakey, Or Gerlitz
In-Reply-To: <1495718679-20693-1-git-send-email-ogerlitz@mellanox.com>

Add support for dissection of ip tos and ttl and ipv6 traffic-class
and hoplimit. Both are dissected into the same struct.

Uses similar call to ip dissection function as with tcp, arp and others.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/flow_dissector.h | 11 +++++++++++
 net/core/flow_dissector.c    | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index efe34eec..e2663e9 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -165,6 +165,16 @@ struct flow_dissector_key_tcp {
 	__be16 flags;
 };
 
+/**
+ * struct flow_dissector_key_ip:
+ * @tos: tos
+ * @ttl: ttl
+ */
+struct flow_dissector_key_ip {
+	__u8	tos;
+	__u8	ttl;
+};
+
 enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
 	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
@@ -186,6 +196,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_ENC_PORTS, /* struct flow_dissector_key_ports */
 	FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
 	FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
+	FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 5a45943..fc5fc45 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -367,6 +367,40 @@ __skb_flow_dissect_tcp(const struct sk_buff *skb,
 	key_tcp->flags = (*(__be16 *) &tcp_flag_word(th) & htons(0x0FFF));
 }
 
+static void
+__skb_flow_dissect_ipv4(const struct sk_buff *skb,
+			struct flow_dissector *flow_dissector,
+			void *target_container, void *data, const struct iphdr *iph)
+{
+	struct flow_dissector_key_ip *key_ip;
+
+	if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_IP))
+		return;
+
+	key_ip = skb_flow_dissector_target(flow_dissector,
+					   FLOW_DISSECTOR_KEY_IP,
+					   target_container);
+	key_ip->tos = iph->tos;
+	key_ip->ttl = iph->ttl;
+}
+
+static void
+__skb_flow_dissect_ipv6(const struct sk_buff *skb,
+			struct flow_dissector *flow_dissector,
+			void *target_container, void *data, const struct ipv6hdr *iph)
+{
+	struct flow_dissector_key_ip *key_ip;
+
+	if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_IP))
+		return;
+
+	key_ip = skb_flow_dissector_target(flow_dissector,
+					   FLOW_DISSECTOR_KEY_IP,
+					   target_container);
+	key_ip->tos = ipv6_get_dsfield(iph);
+	key_ip->ttl = iph->hop_limit;
+}
+
 /**
  * __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
@@ -469,6 +503,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			}
 		}
 
+		__skb_flow_dissect_ipv4(skb, flow_dissector,
+					target_container, data, iph);
+
 		if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
 			goto out_good;
 
@@ -514,6 +551,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 				goto out_good;
 		}
 
+		__skb_flow_dissect_ipv6(skb, flow_dissector,
+					target_container, data, iph);
+
 		if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
 			goto out_good;
 
-- 
2.3.7

^ permalink raw reply related

* [PATCH net-next 4/4] net/mlx5e: Offload TC matching on ip tos / traffic-class
From: Or Gerlitz @ 2017-05-25 13:24 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Saeed Mahameed, Roi Dayan, Paul Blakey, Or Gerlitz
In-Reply-To: <1495718679-20693-1-git-send-email-ogerlitz@mellanox.com>

Enable offloading of TC matching on ipv4 tos or ipv6 traffic-class.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 26 ++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 8e856cd..f8403fd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -581,7 +581,8 @@ static int __parse_cls_flower(struct mlx5e_priv *priv,
 	      BIT(FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS) |
 	      BIT(FLOW_DISSECTOR_KEY_ENC_PORTS)	|
 	      BIT(FLOW_DISSECTOR_KEY_ENC_CONTROL) |
-	      BIT(FLOW_DISSECTOR_KEY_TCP))) {
+	      BIT(FLOW_DISSECTOR_KEY_TCP) |
+	      BIT(FLOW_DISSECTOR_KEY_IP))) {
 		netdev_warn(priv->netdev, "Unsupported key used: 0x%x\n",
 			    f->dissector->used_keys);
 		return -EOPNOTSUPP;
@@ -808,6 +809,29 @@ static int __parse_cls_flower(struct mlx5e_priv *priv,
 			*min_inline = MLX5_INLINE_MODE_TCP_UDP;
 	}
 
+	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_IP)) {
+		struct flow_dissector_key_ip *key =
+			skb_flow_dissector_target(f->dissector,
+						  FLOW_DISSECTOR_KEY_IP,
+						  f->key);
+		struct flow_dissector_key_ip *mask =
+			skb_flow_dissector_target(f->dissector,
+						  FLOW_DISSECTOR_KEY_IP,
+						  f->mask);
+
+		MLX5_SET(fte_match_set_lyr_2_4, headers_c, ip_ecn, mask->tos & 0x3);
+		MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_ecn, key->tos & 0x3);
+
+		MLX5_SET(fte_match_set_lyr_2_4, headers_c, ip_dscp, mask->tos >> 2);
+		MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_dscp, key->tos  >> 2);
+
+		if (mask->tos)
+			*min_inline = MLX5_INLINE_MODE_IP;
+
+		if (mask->ttl) /* currently not supported */
+			return -EOPNOTSUPP;
+	}
+
 	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_TCP)) {
 		struct flow_dissector_key_tcp *key =
 			skb_flow_dissector_target(f->dissector,
-- 
2.3.7

^ permalink raw reply related

* [PATCH net V2] net/mlx4: Fix the check in attaching steering rules
From: Tariq Toukan @ 2017-05-25 13:21 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Eran Ben Elisha, Or Gerlitz, Talat Batheesh, Tariq Toukan

From: Talat Batheesh <talatb@mellanox.com>

Our previous patch (cited below) introduced a regression
for RAW Eth QPs.

Fix it by checking if the QP number provided by user-space
exists, hence allowing steering rules to be added for valid
QPs only.

Fixes: 89c557687a32 ("net/mlx4_en: Avoid adding steering rules with ...")
Reported-by: Or Gerlitz <gerlitz.or@gmail.com>
Signed-off-by: Talat Batheesh <talatb@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Acked-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
---
v2:
* Removed unnecessary EXPORT_SYMBOL_GPL.

 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |  3 +--
 drivers/net/ethernet/mellanox/mlx4/qp.c         | 13 +++++++++++++
 include/linux/mlx4/qp.h                         |  1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index ae5fdc2df654..00a7cd3dcc2e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -1562,8 +1562,7 @@ static int mlx4_en_flow_replace(struct net_device *dev,
 		qpn = priv->drop_qp.qpn;
 	else if (cmd->fs.ring_cookie & EN_ETHTOOL_QP_ATTACH) {
 		qpn = cmd->fs.ring_cookie & (EN_ETHTOOL_QP_ATTACH - 1);
-		if (qpn < priv->rss_map.base_qpn ||
-		    qpn >= priv->rss_map.base_qpn + priv->rx_ring_num) {
+		if (!mlx4_qp_lookup(priv->mdev->dev, qpn)) {
 			en_warn(priv, "rxnfc: QP (0x%x) doesn't exist\n", qpn);
 			return -EINVAL;
 		}
diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c b/drivers/net/ethernet/mellanox/mlx4/qp.c
index 2d6abd4662b1..ad92d2311478 100644
--- a/drivers/net/ethernet/mellanox/mlx4/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
@@ -384,6 +384,19 @@ static void mlx4_qp_free_icm(struct mlx4_dev *dev, int qpn)
 		__mlx4_qp_free_icm(dev, qpn);
 }
 
+struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn)
+{
+	struct mlx4_qp_table *qp_table = &mlx4_priv(dev)->qp_table;
+	struct mlx4_qp *qp;
+
+	spin_lock(&qp_table->lock);
+
+	qp = __mlx4_qp_lookup(dev, qpn);
+
+	spin_unlock(&qp_table->lock);
+	return qp;
+}
+
 int mlx4_qp_alloc(struct mlx4_dev *dev, int qpn, struct mlx4_qp *qp, gfp_t gfp)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
index b4ee8f62ce8d..8e2828d48d7f 100644
--- a/include/linux/mlx4/qp.h
+++ b/include/linux/mlx4/qp.h
@@ -470,6 +470,7 @@ struct mlx4_update_qp_params {
 	u16	rate_val;
 };
 
+struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn);
 int mlx4_update_qp(struct mlx4_dev *dev, u32 qpn,
 		   enum mlx4_update_qp_attr attr,
 		   struct mlx4_update_qp_params *params);
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net] net/mlx4: Fix the check in attaching steering rules
From: Tariq Toukan @ 2017-05-25 13:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, eranbe, ogerlitz, talatb
In-Reply-To: <20170524.153625.1291847374941790972.davem@davemloft.net>


On 24/05/2017 10:36 PM, David Miller wrote:
> From: Tariq Toukan <tariqt@mellanox.com>
> Date: Tue, 23 May 2017 15:50:07 +0300
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>> index ae5fdc2df654..00a7cd3dcc2e 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>> @@ -1562,8 +1562,7 @@ static int mlx4_en_flow_replace(struct net_device *dev,
>>   		qpn = priv->drop_qp.qpn;
>>   	else if (cmd->fs.ring_cookie & EN_ETHTOOL_QP_ATTACH) {
>>   		qpn = cmd->fs.ring_cookie & (EN_ETHTOOL_QP_ATTACH - 1);
>> -		if (qpn < priv->rss_map.base_qpn ||
>> -		    qpn >= priv->rss_map.base_qpn + priv->rx_ring_num) {
>> +		if (!mlx4_qp_lookup(priv->mdev->dev, qpn)) {
>>   			en_warn(priv, "rxnfc: QP (0x%x) doesn't exist\n", qpn);
>>   			return -EINVAL;
>>   		}
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c b/drivers/net/ethernet/mellanox/mlx4/qp.c
>> index 2d6abd4662b1..1eff2fe32a8b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/qp.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
>> @@ -384,6 +384,20 @@ static void mlx4_qp_free_icm(struct mlx4_dev *dev, int qpn)
>>   		__mlx4_qp_free_icm(dev, qpn);
>>   }
>>   
>> +struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn)
>   ...
>> +EXPORT_SYMBOL_GPL(mlx4_qp_lookup);
>> +
>
> This phony separation between MLX4_CORE and MLX4_EN is the only reason
> you need this unreasonable symbol export.
>
> I doubt you'll ever use this function anywhere outside of en_ethtool.c
> so this export is wasted space in the kernel image.  Probably compiler
> could inline it decently as well.
>
> So find another way to do this without the symbol export.  I don't
> really want to hear any stories about "clean separation" or whatever.
> What's happening here is exactly why this separate modules scheme
> results in ugly unreasonable code, and unnecessary gymnastics and
> wasted object space just to make routines available in one place from
> another.

I see. I'll remove this EXPORT_SYMBOL_GPL and send a re-spin shortly.
Function will still be accessible within EN driver, as mlx4_en.h includes qp.h.

Regards,
Tariq

^ permalink raw reply

* RE: [PATCH] dt-bindings: net: move FMan binding
From: Madalin-Cristian Bucur @ 2017-05-25 12:57 UTC (permalink / raw)
  To: David Miller, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org,
	oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org,
	paulus-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org,
	mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170515.103048.248884533404807350.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

> -----Original Message-----
> From: David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
> Sent: Monday, May 15, 2017 5:31 PM
> Subject: Re: [PATCH] dt-bindings: net: move FMan binding
> 
> From: Madalin Bucur <madalin.bucur-3arQi8VN3Tc@public.gmane.org>
> Date: Mon, 15 May 2017 16:36:38 +0300
> 
> > Besides the PPC SoCs, the QorIQ DPAA FMan is also present on ARM SoCs,
> > moving the device tree binding document into the bindings/net folder.
> >
> > Signed-off-by: Madalin Bucur <madalin.bucur-3arQi8VN3Tc@public.gmane.org>
> 
> What tree is this targetted at for merging?

I hope Rob Herring will take this into his tree, it's a device tree binding
change. 

Thanks,
Madalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v9 06/15] mlx5: Replace PCI pool old API
From: Romain Perier @ 2017-05-25 11:37 UTC (permalink / raw)
  To: David Miller
  Cc: leon, dan.j.williams, dledford, sean.hefty, hal.rosenstock,
	jeffrey.t.kirsher, stas.yakovlev, jejb, martin.petersen, balbi,
	gregkh, linux-rdma, netdev, linux-usb, linux-scsi, linux-kernel,
	peter.senna
In-Reply-To: <20170523.112658.1479025487863881513.davem@davemloft.net>

Hello,


Le 23/05/2017 à 17:26, David Miller a écrit :
> From: Romain Perier <romain.perier@collabora.com>
> Date: Tue, 23 May 2017 10:53:36 +0200
>
>> Hello,
>>
>>
>> Le 23/05/2017 à 09:27, Leon Romanovsky a écrit :
>>> On Mon, May 22, 2017 at 06:48:58PM +0200, Romain Perier wrote:
>>>> The PCI pool API is deprecated. This commit replaces the PCI pool old
>>>> API by the appropriate function with the DMA pool API.
>>>>
>>>> Signed-off-by: Romain Perier <romain.perier@collabora.com>
>>>> Reviewed-by: Peter Senna Tschudin <peter.senna@collabora.com>
>>>> Acked-by: Doug Ledford <dledford@redhat.com>
>>>> Tested-by: Doug Ledford <dledford@redhat.com>
>>>> ---
>>>>  drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 11 ++++++-----
>>>>  include/linux/mlx5/driver.h                   |  2 +-
>>>>  2 files changed, 7 insertions(+), 6 deletions(-)
>>>>
>>> Who is supposed to merge this patch series?
>>>
>>> Acked-by: Leon Romanovsky <leonro@mellanox.com>
>> Each maintainer of the corresponding subsystem, can take a patch, I
>> guess. No ?
> It might be easier to accumulate acks and you submit them as a series,
> in my opinion.
That's what I did already, no ?

Romain

^ permalink raw reply

* Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Saeed Mahameed @ 2017-05-25 10:40 UTC (permalink / raw)
  To: Ilan Tayari
  Cc: Alexei Starovoitov, Saeed Mahameed, David S. Miller, Doug Ledford,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jsorensen-b10kYP2dOMg@public.gmane.org
In-Reply-To: <AM4PR0501MB1940885C2F1CEF4DDE4EA8D1DBFF0-dp/nxUn679gfNUYDR5dMTsDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>

On Thu, May 25, 2017 at 8:20 AM, Ilan Tayari <ilant-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> -----Original Message-----
>> From: Alexei Starovoitov [mailto:alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>>
>> On Tue, May 23, 2017 at 02:44:02PM +0300, Saeed Mahameed wrote:
>> > From: Ilan Tayari <ilant-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >
>> > Mellanox Innova is a NIC with ConnectX and an FPGA on the same
>> > board. The FPGA is a bump-on-the-wire and thus affects operation of
>> > the mlx5_core driver on the ConnectX ASIC.
>> >
>> > Add basic support for Innova in mlx5_core.
>> >
>> > This allows using the Innova card as a regular NIC, by detecting
>> > the FPGA capability bit, and verifying its load state before
>> > initializing ConnectX interfaces.
>>
>> That doesn't sound like cx4/cx5 hw that mlx5 driver suppose to support.
>
> Hi Alexei,
> Thanks for your feedback.
> Let me address your concerns.
>
> I didn't state it, but the current iterations of Innova cards include the
> ConnectX-4LX chip, which is exactly what mlx5 driver supports.
>
> The PCI interface is *only* through the ConnectX chip with some new
> Firmware commands. This patch doesn't register any new PCI device,
> so it does not make sense to create a separate driver.
>
>>
>> > Also detect FPGA fatal runtime failures and enter error state if
>> > they ever happen.
>> >
>> > All new FPGA-related logic is placed in its own subdirectory 'fpga',
>> > which may be built by selecting CONFIG_MLX5_FPGA.
>> > This prepares for further support of various Innova features in later
>> > patchsets.
>> > Additional details about hardware architecture will be provided as
>> > more features get submitted.
>> >
>> > Signed-off-by: Ilan Tayari <ilant-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > Reviewed-by: Boris Pismenny <borisp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > Signed-off-by: Saeed Mahameed <saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > ---
>> >  MAINTAINERS                                        |  10 +
>> >  drivers/net/ethernet/mellanox/mlx5/core/Kconfig    |  10 +
>> >  drivers/net/ethernet/mellanox/mlx5/core/Makefile   |   3 +
>> >  drivers/net/ethernet/mellanox/mlx5/core/eq.c       |  11 ++
>> >  drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.c |  64 +++++++
>> >  drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.h |  59 ++++++
>> >  .../net/ethernet/mellanox/mlx5/core/fpga/core.c    | 202
>> +++++++++++++++++++++
>> >  .../net/ethernet/mellanox/mlx5/core/fpga/core.h    |  99 ++++++++++
>> >  drivers/net/ethernet/mellanox/mlx5/core/main.c     |  19 +-
>> >  include/linux/mlx5/device.h                        |   6 +
>> >  include/linux/mlx5/driver.h                        |   5 +
>> >  include/linux/mlx5/mlx5_ifc.h                      |  11 +-
>> >  include/linux/mlx5/mlx5_ifc_fpga.h                 | 144
>> +++++++++++++++
>> >  13 files changed, 640 insertions(+), 3 deletions(-)
>>
>> Can you put it into different driver? Dumping everything into by far
>> the biggest nic driver already is already huge headache in terms on
>> maintainability, debugging and back ports.
>> Look at how intel splits their drivers.
>> ixgb, ixgbe, ixgbevf are different drivers thought they have a lot in
>> common. On one side it's a bit of copy paste, but on the other side

I don't think the ixgb example is the same, simply  ixgb, ixgbe,
ixgbevf have different PCI IDs
and even different SW/FW interfaces. On the other hand, same mlx5
driver can support all of
ConnetX4/5/6 device IDs with the same code flows, same interfaces.

>> it makes drivers much easier to develop and maintain independently.
>> ConnectX-6 code and any future hw support doesn't belong to
>> mlx5 driver at all.

Sorry i must disagree with you on this for the same reasons Ilan mentioned.
We can perfectly achieve the same with modular driver design all under the
same .ko module, with some kconfig flags to reduce the amount of code/features
this .ko provides.

>
> If you build your driver without explicitly enabling CONFIG_MLX5_FPGA=y
> (default is N) then you get none of this, and your driver is practically
> the same as before.
>
> The FPGA and CX operation is very tightly integrated.
> If you don't want any of this, simply don't opt-in to CONFIG_MLX5_FPGA.
>
> If you do want this, then splitting some of the logic to a
> separate kernel object will not gain anything useful (logic would stay
> the same), and just pollute the exported symbol table and open up the door
> for issues of inter-module compatibility and so on.
>
> Furthermore, the next patchset will introduce IPSec offload capabilities
> Which are declared in netdev->hw_features. Those cannot be modified
> after the netdevice is created, so all the extra logic has to be
> integrated into the mlx5 ethernet driver. This is another reason why
> a separate driver is a bad idea.
>
> We will include details about the board and how things work in the
> cover letter of the IPSec offload patchset.
>
> Ilan.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] i40e: Fix incorrect pf->flags
From: Jeff Kirsher @ 2017-05-25  9:13 UTC (permalink / raw)
  To: Tushar Dave, intel-wired-lan, netdev
In-Reply-To: <1495242112-14835-1-git-send-email-tushar.n.dave@oracle.com>

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

On Fri, 2017-05-19 at 18:01 -0700, Tushar Dave wrote:
> Fix bug introduced by 'commit 47994c119a36e ("i40e: remove
> hw_disabled_flags in favor of using separate flag bits")' that
> mistakenly wipes out pf->flags.
> 
> Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

:-(
Not even close to applying, I even tried to apply by hand but the patch
was not near the same as my branch in my tree.  I will forgive this
second instance of a patch generated on different tree/branch, but I
really must insist that you use my tree & branch if you need/want to
make changes to Intel wired LAN driver changes.

> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index d5c9c9e..6b98d34 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -8821,9 +8821,9 @@ static int i40e_sw_init(struct i40e_pf *pf)
>  		    (pf->hw.aq.api_min_ver > 4))) {
>  		/* Supported in FW API version higher than 1.4 */
>  		pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
> -		pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> +		pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
>  	} else {
> -		pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> +		pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
>  	}
>  
>  	pf->eeprom_version = 0xDEAD;

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 2/4] nfp: register ports as devlink ports
From: Jiri Pirko @ 2017-05-25  8:20 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20170524122413.702e0eef@cakuba.netronome.com>

Wed, May 24, 2017 at 09:24:13PM CEST, kubakici@wp.pl wrote:
>On Wed, 24 May 2017 14:35:14 +0200, Jiri Pirko wrote:
>> >+void nfp_devlink_port_unregister(struct nfp_port *port)
>> >+{
>> >+	/* Due to unpleasant lock ordering we may see the port go away before
>> >+	 * we have fully probed.  
>> 
>> Could you elaborate on this a bit more please?
>
>It's partially due to peculiarities of the management FW more than
>kernel stuff.  Unfortunately some ethtool media config requires reboot
>to be applied, so we print a friendly message to the logs and
>unregister the associated netdevs.  Which means once netdevs get
>registered ports may go away.
>
>Enter devlink, I need the ability to grab the adapater lock in
>split/unsplit callbacks to find the ports, which implies having to drop
>that lock before I register devlink.  And only after I register devlink
>can I register the ports.
>
>I could do init without registering anything, drop the adapter lock,
>register devlink, and then grab the adapter lock back and register
>devlink ports and netdevs.  But there is another issue...
>
>Since I look for ports on a list maintained in the adapter struct,
>driver code doesn't care if devlink_port has been registered or not.
>The moment devlink is registered, split/unsplit requests will be
>accepted - potentially trying to unregister devlink_port before the
>register could happen.
>
>Further down the line, also, the eswitch mode setting is coming.  Which
>means the moment I register devlink itself ports will get shuffled (due
>to the plan of registering VFs as ports :)). 
>
>I feel like registering devlink should be the last action of the
>driver, really.  My plan was to keep that simple if() for now, and once
>we get to extending devlink with SR-IOV stuff also add the ability to
>pre-register ports.  Allow registering ports on not-yet-registered
>devlink (probably put them on a private list within struct devlink).
>This would make devlink_register() a single point when everything
>devlink becomes visible, atomically, instead of devlink itself coming
>first and then ports following.
>
>Does that make sense?  Am I misreading the code (again :S)?

Well in mlxsw, we internally maintain a list of port and we assign a
pointer to the port struct only after all is initialized:

...
mlxsw_sp->ports[local_port] = mlxsw_sp_port;
err = register_netdev(dev);
...

Then in split, we check:
mlxsw_sp_port = mlxsw_sp->ports[local_port];
if (!mlxsw_sp_port) {
        dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
                local_port);
        return -EINVAL;
}

I guess that you can do something similar. You should ensure that split
won't happen for half-initialized ports.

^ permalink raw reply

* Re: [patch net-next v2 0/8] mlxsw: Support firmware flash
From: Jiri Pirko @ 2017-05-25  6:15 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, idosch, yotamg, mlxsw, Yuval.Mintz
In-Reply-To: <20170524.163328.672261790569871029.davem@davemloft.net>

Wed, May 24, 2017 at 10:33:28PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Tue, 23 May 2017 21:56:22 +0200
>
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Add support for device firmware flash on mlxsw spectrum. The firmware files
>> are expected to be in the Mellanox Firmware Archive version 2 (MFA2)
>> format.
>> 
>> The firmware flash is triggered on driver initialization time if the device
>> firmware version does not meet the minimum firmware version supported by
>> the driver.
>> 
>> Currently, to activate the newly flashed firmware, the user needs to
>> reboot his system.
>> 
>> The first patch introduces the mlxfw module, which implements common logic
>> needed for the firmware flash process on Mellanox products, such as the
>> MFA2 format parsing and the firmware flash state machine logic. As the
>> module implements common logic which will be needed by various different
>> Mellanox drivers, it defines a set of callbacks needed to interact with the
>> specific device.
>> 
>> Patches 1-5 implement the needed mlxfw callbacks in the mlxsw spectrum
>> driver.
>> 
>> Patches 6 and 7 add boot-time firmware upgrade on the mlxsw spectrum
>> driver.
>> 
>> Patch 8 adds a fix needed for new firmware versions.
>
>Series applied, although I hope you sort out the user interface
>soon otherwise most of this is completely dead unused code.

No dead code. The flashing is used internally from the driver on the
modprobe time. We are looking into the user interface at the moment.
Thanks.

>
>Thanks.

^ permalink raw reply

* [PATCH] net: llc: add lock_sock in llc_ui_bind to avoid a race condition
From: Lin Zhang @ 2017-05-25  6:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, Lin Zhang

There is a race condition in llc_ui_bind if two or more processes/threads 
try to bind a same socket.

If more processes/threads bind a same socket success that will lead to 
two problems, one is this action is not what we expected, another is 
will lead to kernel in unstable status or oops(in my simple test case, 
cause llc2.ko can't unload).

The current code is test SOCK_ZAPPED bit to avoid a process to 
bind a same socket twice but that is can't avoid more processes/threads 
try to bind a same socket at the same time.

So, add lock_sock in llc_ui_bind like others, such as llc_ui_connect.

Signed-off-by: Lin Zhang <xiaolou4617@gmail.com>
---
 net/llc/af_llc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 8364fe5..c38d16f 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -311,6 +311,8 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
 	int rc = -EINVAL;
 
 	dprintk("%s: binding %02X\n", __func__, addr->sllc_sap);
+
+	lock_sock(sk);
 	if (unlikely(!sock_flag(sk, SOCK_ZAPPED) || addrlen != sizeof(*addr)))
 		goto out;
 	rc = -EAFNOSUPPORT;
@@ -382,6 +384,7 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
 out_put:
 	llc_sap_put(sap);
 out:
+	release_sock(sk);
 	return rc;
 }
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] bonding: Don't update slave->link until ready to commit
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-05-25  5:38 UTC (permalink / raw)
  To: Nithin Nayak Sujir; +Cc: David Miller, linux-netdev, Jay Vosburgh
In-Reply-To: <1495680317-11308-1-git-send-email-nsujir@tintri.com>

On Wed, May 24, 2017 at 7:45 PM, Nithin Nayak Sujir <nsujir@tintri.com> wrote:
> In the loadbalance arp monitoring scheme, when a slave link change is
> detected, the slave->link is immediately updated and slave_state_changed
> is set. Later down the function, the rtnl_lock is acquired and the
> changes are committed, updating the bond link state.
>
> However, the acquisition of the rtnl_lock can fail. The next time the
> monitor runs, since slave->link is already updated, it determines that
> link is unchanged. This results in the bond link state permanently out
> of sync with the slave link.
>
> This patch modifies bond_loadbalance_arp_mon() to handle link changes
> identical to bond_ab_arp_{inspect/commit}(). The new link state is
> maintained in slave->new_link until we're ready to commit at which point
> it's copied into slave->link.
>
> NOTE: miimon_{inspect/commit}() has a more complex state machine
> requiring the use of the bond_{propose,commit}_link_state() functions
> which maintains the intermediate state in slave->link_new_state. The arp
> monitors don't require that.
>
> Testing: This bug is very easy to reproduce with the following steps.
> 1. In a loop, toggle a slave link of a bond slave interface.
> 2. In a separate loop, do ifconfig up/down of an unrelated interface to
> create contention for rtnl_lock.
> Within a few iterations, the bond link goes out of sync with the slave
> link.
>
> Signed-off-by: Nithin Nayak Sujir <nsujir@tintri.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
> Cc: Mahesh Bandewar <maheshb@google.com>
> Cc: Jay Vosburgh <jay.vosburgh@canonical.com>
> ---
>  drivers/net/bonding/bond_main.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7331331..2359478b 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2612,11 +2612,13 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
>         bond_for_each_slave_rcu(bond, slave, iter) {
>                 unsigned long trans_start = dev_trans_start(slave->dev);
>
> +               slave->new_link = BOND_LINK_NOCHANGE;
> +
>                 if (slave->link != BOND_LINK_UP) {
>                         if (bond_time_in_interval(bond, trans_start, 1) &&
>                             bond_time_in_interval(bond, slave->last_rx, 1)) {
>
> -                               slave->link  = BOND_LINK_UP;
> +                               slave->new_link = BOND_LINK_UP;
>                                 slave_state_changed = 1;
>
>                                 /* primary_slave has no meaning in round-robin
> @@ -2643,7 +2645,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
>                         if (!bond_time_in_interval(bond, trans_start, 2) ||
>                             !bond_time_in_interval(bond, slave->last_rx, 2)) {
>
> -                               slave->link  = BOND_LINK_DOWN;
> +                               slave->new_link = BOND_LINK_DOWN;
>                                 slave_state_changed = 1;
>
>                                 if (slave->link_failure_count < UINT_MAX)
> @@ -2674,6 +2676,11 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
>                 if (!rtnl_trylock())
>                         goto re_arm;
>
> +               bond_for_each_slave(bond, slave, iter) {
> +                       if (slave->new_link != BOND_LINK_NOCHANGE)
> +                               slave->link = slave->new_link;
> +               }
> +
>                 if (slave_state_changed) {
>                         bond_slave_state_change(bond);
>                         if (BOND_MODE(bond) == BOND_MODE_XOR)
> --
> 2.8.2
>

^ permalink raw reply

* RE: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Ilan Tayari @ 2017-05-25  5:20 UTC (permalink / raw)
  To: Alexei Starovoitov, Saeed Mahameed
  Cc: David S. Miller, Doug Ledford,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jsorensen-b10kYP2dOMg@public.gmane.org
In-Reply-To: <20170524165059.42lmrwzzcxvi2sf7@ast-mbp>

> -----Original Message-----
> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> 
> On Tue, May 23, 2017 at 02:44:02PM +0300, Saeed Mahameed wrote:
> > From: Ilan Tayari <ilant@mellanox.com>
> >
> > Mellanox Innova is a NIC with ConnectX and an FPGA on the same
> > board. The FPGA is a bump-on-the-wire and thus affects operation of
> > the mlx5_core driver on the ConnectX ASIC.
> >
> > Add basic support for Innova in mlx5_core.
> >
> > This allows using the Innova card as a regular NIC, by detecting
> > the FPGA capability bit, and verifying its load state before
> > initializing ConnectX interfaces.
> 
> That doesn't sound like cx4/cx5 hw that mlx5 driver suppose to support.

Hi Alexei,
Thanks for your feedback.
Let me address your concerns.

I didn't state it, but the current iterations of Innova cards include the
ConnectX-4LX chip, which is exactly what mlx5 driver supports.

The PCI interface is *only* through the ConnectX chip with some new
Firmware commands. This patch doesn't register any new PCI device, 
so it does not make sense to create a separate driver.

> 
> > Also detect FPGA fatal runtime failures and enter error state if
> > they ever happen.
> >
> > All new FPGA-related logic is placed in its own subdirectory 'fpga',
> > which may be built by selecting CONFIG_MLX5_FPGA.
> > This prepares for further support of various Innova features in later
> > patchsets.
> > Additional details about hardware architecture will be provided as
> > more features get submitted.
> >
> > Signed-off-by: Ilan Tayari <ilant@mellanox.com>
> > Reviewed-by: Boris Pismenny <borisp@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> >  MAINTAINERS                                        |  10 +
> >  drivers/net/ethernet/mellanox/mlx5/core/Kconfig    |  10 +
> >  drivers/net/ethernet/mellanox/mlx5/core/Makefile   |   3 +
> >  drivers/net/ethernet/mellanox/mlx5/core/eq.c       |  11 ++
> >  drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.c |  64 +++++++
> >  drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.h |  59 ++++++
> >  .../net/ethernet/mellanox/mlx5/core/fpga/core.c    | 202
> +++++++++++++++++++++
> >  .../net/ethernet/mellanox/mlx5/core/fpga/core.h    |  99 ++++++++++
> >  drivers/net/ethernet/mellanox/mlx5/core/main.c     |  19 +-
> >  include/linux/mlx5/device.h                        |   6 +
> >  include/linux/mlx5/driver.h                        |   5 +
> >  include/linux/mlx5/mlx5_ifc.h                      |  11 +-
> >  include/linux/mlx5/mlx5_ifc_fpga.h                 | 144
> +++++++++++++++
> >  13 files changed, 640 insertions(+), 3 deletions(-)
> 
> Can you put it into different driver? Dumping everything into by far
> the biggest nic driver already is already huge headache in terms on
> maintainability, debugging and back ports.
> Look at how intel splits their drivers.
> ixgb, ixgbe, ixgbevf are different drivers thought they have a lot in
> common. On one side it's a bit of copy paste, but on the other side
> it makes drivers much easier to develop and maintain independently.
> ConnectX-6 code and any future hw support doesn't belong to
> mlx5 driver at all.

If you build your driver without explicitly enabling CONFIG_MLX5_FPGA=y
(default is N) then you get none of this, and your driver is practically
the same as before.

The FPGA and CX operation is very tightly integrated.
If you don't want any of this, simply don't opt-in to CONFIG_MLX5_FPGA.

If you do want this, then splitting some of the logic to a
separate kernel object will not gain anything useful (logic would stay
the same), and just pollute the exported symbol table and open up the door
for issues of inter-module compatibility and so on.

Furthermore, the next patchset will introduce IPSec offload capabilities
Which are declared in netdev->hw_features. Those cannot be modified
after the netdevice is created, so all the extra logic has to be 
integrated into the mlx5 ethernet driver. This is another reason why
a separate driver is a bad idea.

We will include details about the board and how things work in the
cover letter of the IPSec offload patchset.

Ilan.

^ permalink raw reply

* Re: [PATCH net-next] ibmvnic: Enable TSO support
From: kbuild test robot @ 2017-05-25  3:57 UTC (permalink / raw)
  To: Thomas Falcon
  Cc: kbuild-all, netdev, Thomas Falcon, Nathan Fontenot, John Allen
In-Reply-To: <1495679366-7149-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

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

Hi Thomas,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Thomas-Falcon/ibmvnic-Enable-TSO-support/20170525-111039
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All warnings (new ones prefixed by >>):

   In file included from include/linux/module.h:18:0,
                    from drivers/net/ethernet/ibm/ibmvnic.c:46:
   drivers/net/ethernet/ibm/ibmvnic.c: In function '__check_large_send_offload':
   include/linux/moduleparam.h:148:27: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     param_check_##type(name, &(value));       \
                              ^
   include/linux/moduleparam.h:346:68: note: in definition of macro '__param_check'
     static inline type __always_unused *__check_##name(void) { return(p); }
                                                                       ^
   include/linux/moduleparam.h:148:2: note: in expansion of macro 'param_check_bool'
     param_check_##type(name, &(value));       \
     ^~~~~~~~~~~~
   include/linux/moduleparam.h:128:2: note: in expansion of macro 'module_param_named'
     module_param_named(name, name, type, perm)
     ^~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/ibm/ibmvnic.c:85:1: note: in expansion of macro 'module_param'
    module_param(large_send_offload, bool, 0644);
    ^~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/module_param +85 drivers/net/ethernet/ibm/ibmvnic.c

    69	#include <net/net_namespace.h>
    70	#include <asm/hvcall.h>
    71	#include <linux/atomic.h>
    72	#include <asm/vio.h>
    73	#include <asm/iommu.h>
    74	#include <linux/uaccess.h>
    75	#include <asm/firmware.h>
    76	#include <linux/workqueue.h>
    77	#include <linux/if_vlan.h>
    78	
    79	#include "ibmvnic.h"
    80	
    81	static const char ibmvnic_driver_name[] = "ibmvnic";
    82	static const char ibmvnic_driver_string[] = "IBM System i/p Virtual NIC Driver";
    83	
    84	static int large_send_offload;
  > 85	module_param(large_send_offload, bool, 0644);
    86	MODULE_PARM_DESC(large_send_offload,
    87			 "Determines whether large send offload is enabled");
    88	
    89	MODULE_AUTHOR("Santiago Leon <santi_leon@yahoo.com>");
    90	MODULE_DESCRIPTION("IBM System i/p Virtual NIC Driver");
    91	MODULE_LICENSE("GPL");
    92	MODULE_VERSION(IBMVNIC_DRIVER_VERSION);
    93	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53754 bytes --]

^ permalink raw reply

* [PATCH net-next 6/6] net: mpls: minor cleanups
From: David Ahern @ 2017-05-25  3:54 UTC (permalink / raw)
  To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>

Noticed these doing the extack support:
- nla_get_via is only used in af_mpls.c so remove from internal.h

- err is initialized to EINVAL in mpls_nh_build_from_cfg but then
  set again before it is checked. Remove the EINVAL setting

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/mpls/af_mpls.c  | 5 +++--
 net/mpls/internal.h | 2 --
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index d0120f8c8a2c..ee468feb9e64 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -43,6 +43,9 @@ static void rtmsg_lfib(int event, u32 label, struct mpls_route *rt,
 		       struct nlmsghdr *nlh, struct net *net, u32 portid,
 		       unsigned int nlm_flags);
 
+static int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
+		       u8 via[], struct netlink_ext_ack *extack);
+
 static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
 {
 	struct mpls_route *rt = NULL;
@@ -695,8 +698,6 @@ static int mpls_nh_build_from_cfg(struct mpls_route_config *cfg,
 	if (!nh)
 		return -ENOMEM;
 
-	err = -EINVAL;
-
 	nh->nh_labels = cfg->rc_output_labels;
 	for (i = 0; i < nh->nh_labels; i++)
 		nh->nh_label[i] = cfg->rc_output_label[i];
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index a015a6a1143b..cf65aec2e551 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -204,8 +204,6 @@ int nla_put_labels(struct sk_buff *skb, int attrtype,  u8 labels,
 		   const u32 label[]);
 int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
 		   u32 label[], struct netlink_ext_ack *extack);
-int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
-		u8 via[], struct netlink_ext_ack *extack);
 bool mpls_output_possible(const struct net_device *dev);
 unsigned int mpls_dev_mtu(const struct net_device *dev);
 bool mpls_pkt_too_big(const struct sk_buff *skb, unsigned int mtu);
-- 
2.11.0 (Apple Git-81)

^ permalink raw reply related

* [PATCH net-next 5/6] net: mpls: Add extack messages for route add and delete failures
From: David Ahern @ 2017-05-25  3:54 UTC (permalink / raw)
  To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>

Add error messages for failures in adding and deleting mpls routes.
This covers most of the annoying EINVAL errors.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/mpls/af_mpls.c  | 136 +++++++++++++++++++++++++++++++++++++---------------
 net/mpls/internal.h |   2 +-
 2 files changed, 98 insertions(+), 40 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index f3830951fb1c..d0120f8c8a2c 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -720,7 +720,8 @@ static int mpls_nh_build_from_cfg(struct mpls_route_config *cfg,
 
 static int mpls_nh_build(struct net *net, struct mpls_route *rt,
 			 struct mpls_nh *nh, int oif, struct nlattr *via,
-			 struct nlattr *newdst, u8 max_labels)
+			 struct nlattr *newdst, u8 max_labels,
+			 struct netlink_ext_ack *extack)
 {
 	int err = -ENOMEM;
 
@@ -729,14 +730,14 @@ static int mpls_nh_build(struct net *net, struct mpls_route *rt,
 
 	if (newdst) {
 		err = nla_get_labels(newdst, max_labels, &nh->nh_labels,
-				     nh->nh_label, NULL);
+				     nh->nh_label, extack);
 		if (err)
 			goto errout;
 	}
 
 	if (via) {
 		err = nla_get_via(via, &nh->nh_via_alen, &nh->nh_via_table,
-				  __mpls_nh_via(rt, nh));
+				  __mpls_nh_via(rt, nh), extack);
 		if (err)
 			goto errout;
 	} else {
@@ -803,7 +804,8 @@ static u8 mpls_count_nexthops(struct rtnexthop *rtnh, int len,
 }
 
 static int mpls_nh_build_multi(struct mpls_route_config *cfg,
-			       struct mpls_route *rt, u8 max_labels)
+			       struct mpls_route *rt, u8 max_labels,
+			       struct netlink_ext_ack *extack)
 {
 	struct rtnexthop *rtnh = cfg->rc_mp;
 	struct nlattr *nla_via, *nla_newdst;
@@ -837,7 +839,7 @@ static int mpls_nh_build_multi(struct mpls_route_config *cfg,
 
 		err = mpls_nh_build(cfg->rc_nlinfo.nl_net, rt, nh,
 				    rtnh->rtnh_ifindex, nla_via, nla_newdst,
-				    max_labels);
+				    max_labels, extack);
 		if (err)
 			goto errout;
 
@@ -856,7 +858,8 @@ static int mpls_nh_build_multi(struct mpls_route_config *cfg,
 	return err;
 }
 
-static int mpls_route_add(struct mpls_route_config *cfg)
+static int mpls_route_add(struct mpls_route_config *cfg,
+			  struct netlink_ext_ack *extack)
 {
 	struct mpls_route __rcu **platform_label;
 	struct net *net = cfg->rc_nlinfo.nl_net;
@@ -876,17 +879,25 @@ static int mpls_route_add(struct mpls_route_config *cfg)
 	}
 
 	/* Reserved labels may not be set */
-	if (index < MPLS_LABEL_FIRST_UNRESERVED)
+	if (index < MPLS_LABEL_FIRST_UNRESERVED) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid label - must be MPLS_LABEL_FIRST_UNRESERVED or higher");
 		goto errout;
+	}
 
 	/* The full 20 bit range may not be supported. */
-	if (index >= net->mpls.platform_labels)
+	if (index >= net->mpls.platform_labels) {
+		NL_SET_ERR_MSG(extack,
+			       "Label >= configured max in platform_labels");
 		goto errout;
+	}
 
 	/* Append makes no sense with mpls */
 	err = -EOPNOTSUPP;
-	if (cfg->rc_nlflags & NLM_F_APPEND)
+	if (cfg->rc_nlflags & NLM_F_APPEND) {
+		NL_SET_ERR_MSG(extack, "MPLS does not support route append");
 		goto errout;
+	}
 
 	err = -EEXIST;
 	platform_label = rtnl_dereference(net->mpls.platform_label);
@@ -913,8 +924,10 @@ static int mpls_route_add(struct mpls_route_config *cfg)
 		nhs = 1;
 	}
 
-	if (nhs == 0)
+	if (nhs == 0) {
+		NL_SET_ERR_MSG(extack, "Route does not contain a nexthop");
 		goto errout;
+	}
 
 	err = -ENOMEM;
 	rt = mpls_rt_alloc(nhs, max_via_alen, max_labels);
@@ -928,7 +941,7 @@ static int mpls_route_add(struct mpls_route_config *cfg)
 	rt->rt_ttl_propagate = cfg->rc_ttl_propagate;
 
 	if (cfg->rc_mp)
-		err = mpls_nh_build_multi(cfg, rt, max_labels);
+		err = mpls_nh_build_multi(cfg, rt, max_labels, extack);
 	else
 		err = mpls_nh_build_from_cfg(cfg, rt);
 	if (err)
@@ -944,7 +957,8 @@ static int mpls_route_add(struct mpls_route_config *cfg)
 	return err;
 }
 
-static int mpls_route_del(struct mpls_route_config *cfg)
+static int mpls_route_del(struct mpls_route_config *cfg,
+			  struct netlink_ext_ack *extack)
 {
 	struct net *net = cfg->rc_nlinfo.nl_net;
 	unsigned index;
@@ -953,12 +967,18 @@ static int mpls_route_del(struct mpls_route_config *cfg)
 	index = cfg->rc_label;
 
 	/* Reserved labels may not be removed */
-	if (index < MPLS_LABEL_FIRST_UNRESERVED)
+	if (index < MPLS_LABEL_FIRST_UNRESERVED) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid label - must be MPLS_LABEL_FIRST_UNRESERVED or higher");
 		goto errout;
+	}
 
 	/* The full 20 bit range may not be supported */
-	if (index >= net->mpls.platform_labels)
+	if (index >= net->mpls.platform_labels) {
+		NL_SET_ERR_MSG(extack,
+			       "Label >= configured max in platform_labels");
 		goto errout;
+	}
 
 	mpls_route_update(net, index, NULL, &cfg->rc_nlinfo);
 
@@ -1623,19 +1643,25 @@ int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
 }
 EXPORT_SYMBOL_GPL(nla_get_labels);
 
-int nla_get_via(const struct nlattr *nla, u8 *via_alen,
-		u8 *via_table, u8 via_addr[])
+int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
+		u8 via_addr[], struct netlink_ext_ack *extack)
 {
 	struct rtvia *via = nla_data(nla);
 	int err = -EINVAL;
 	int alen;
 
-	if (nla_len(nla) < offsetof(struct rtvia, rtvia_addr))
+	if (nla_len(nla) < offsetof(struct rtvia, rtvia_addr)) {
+		NL_SET_ERR_MSG_ATTR(extack, nla,
+				    "Invalid attribute length for RTA_VIA");
 		goto errout;
+	}
 	alen = nla_len(nla) -
 			offsetof(struct rtvia, rtvia_addr);
-	if (alen > MAX_VIA_ALEN)
+	if (alen > MAX_VIA_ALEN) {
+		NL_SET_ERR_MSG_ATTR(extack, nla,
+				    "Invalid address length for RTA_VIA");
 		goto errout;
+	}
 
 	/* Validate the address family */
 	switch (via->rtvia_family) {
@@ -1665,8 +1691,10 @@ int nla_get_via(const struct nlattr *nla, u8 *via_alen,
 	return err;
 }
 
-static int rtm_to_route_config(struct sk_buff *skb,  struct nlmsghdr *nlh,
-			       struct mpls_route_config *cfg)
+static int rtm_to_route_config(struct sk_buff *skb,
+			       struct nlmsghdr *nlh,
+			       struct mpls_route_config *cfg,
+			       struct netlink_ext_ack *extack)
 {
 	struct rtmsg *rtm;
 	struct nlattr *tb[RTA_MAX+1];
@@ -1674,35 +1702,54 @@ static int rtm_to_route_config(struct sk_buff *skb,  struct nlmsghdr *nlh,
 	int err;
 
 	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_mpls_policy,
-			  NULL);
+			  extack);
 	if (err < 0)
 		goto errout;
 
 	err = -EINVAL;
 	rtm = nlmsg_data(nlh);
 
-	if (rtm->rtm_family != AF_MPLS)
+	if (rtm->rtm_family != AF_MPLS) {
+		NL_SET_ERR_MSG(extack, "Invalid address family in rtmsg");
 		goto errout;
-	if (rtm->rtm_dst_len != 20)
+	}
+	if (rtm->rtm_dst_len != 20) {
+		NL_SET_ERR_MSG(extack, "rtm_dst_len must be 20 for MPLS");
 		goto errout;
-	if (rtm->rtm_src_len != 0)
+	}
+	if (rtm->rtm_src_len != 0) {
+		NL_SET_ERR_MSG(extack, "rtm_src_len must be 0 for MPLS");
 		goto errout;
-	if (rtm->rtm_tos != 0)
+	}
+	if (rtm->rtm_tos != 0) {
+		NL_SET_ERR_MSG(extack, "rtm_tos must be 0 for MPLS");
 		goto errout;
-	if (rtm->rtm_table != RT_TABLE_MAIN)
+	}
+	if (rtm->rtm_table != RT_TABLE_MAIN) {
+		NL_SET_ERR_MSG(extack,
+			       "MPLS only supports the main route table");
 		goto errout;
+	}
 	/* Any value is acceptable for rtm_protocol */
 
 	/* As mpls uses destination specific addresses
 	 * (or source specific address in the case of multicast)
 	 * all addresses have universal scope.
 	 */
-	if (rtm->rtm_scope != RT_SCOPE_UNIVERSE)
+	if (rtm->rtm_scope != RT_SCOPE_UNIVERSE) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid route scope  - MPLS only supports UNIVERSE");
 		goto errout;
-	if (rtm->rtm_type != RTN_UNICAST)
+	}
+	if (rtm->rtm_type != RTN_UNICAST) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid route type - MPLS only supports UNICAST");
 		goto errout;
-	if (rtm->rtm_flags != 0)
+	}
+	if (rtm->rtm_flags != 0) {
+		NL_SET_ERR_MSG(extack, "rtm_flags must be 0 for MPLS");
 		goto errout;
+	}
 
 	cfg->rc_label		= LABEL_NOT_SPECIFIED;
 	cfg->rc_protocol	= rtm->rtm_protocol;
@@ -1725,26 +1772,33 @@ static int rtm_to_route_config(struct sk_buff *skb,  struct nlmsghdr *nlh,
 		case RTA_NEWDST:
 			if (nla_get_labels(nla, MAX_NEW_LABELS,
 					   &cfg->rc_output_labels,
-					   cfg->rc_output_label, NULL))
+					   cfg->rc_output_label, extack)) {
+				NL_SET_BAD_ATTR(extack, nla);
 				goto errout;
+			}
 			break;
 		case RTA_DST:
 		{
 			u8 label_count;
 			if (nla_get_labels(nla, 1, &label_count,
-					   &cfg->rc_label, NULL))
+					   &cfg->rc_label, extack)) {
+				NL_SET_BAD_ATTR(extack, nla);
 				goto errout;
+			}
 
 			/* Reserved labels may not be set */
-			if (cfg->rc_label < MPLS_LABEL_FIRST_UNRESERVED)
+			if (cfg->rc_label < MPLS_LABEL_FIRST_UNRESERVED) {
+				NL_SET_ERR_MSG_ATTR(extack, nla,
+						    "Invalid label for RTA_DST");
 				goto errout;
-
+			}
 			break;
 		}
 		case RTA_VIA:
 		{
 			if (nla_get_via(nla, &cfg->rc_via_alen,
-					&cfg->rc_via_table, cfg->rc_via))
+					&cfg->rc_via_table, cfg->rc_via,
+					extack))
 				goto errout;
 			break;
 		}
@@ -1758,14 +1812,18 @@ static int rtm_to_route_config(struct sk_buff *skb,  struct nlmsghdr *nlh,
 		{
 			u8 ttl_propagate = nla_get_u8(nla);
 
-			if (ttl_propagate > 1)
+			if (ttl_propagate > 1) {
+				NL_SET_ERR_MSG_ATTR(extack, nla,
+						    "RTA_TTL_PROPAGATE can only be 0 or 1");
 				goto errout;
+			}
 			cfg->rc_ttl_propagate = ttl_propagate ?
 				MPLS_TTL_PROP_ENABLED :
 				MPLS_TTL_PROP_DISABLED;
 			break;
 		}
 		default:
+			NL_SET_ERR_MSG_ATTR(extack, nla, "Unknown attribute");
 			/* Unsupported attribute */
 			goto errout;
 		}
@@ -1786,11 +1844,11 @@ static int mpls_rtm_delroute(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (!cfg)
 		return -ENOMEM;
 
-	err = rtm_to_route_config(skb, nlh, cfg);
+	err = rtm_to_route_config(skb, nlh, cfg, extack);
 	if (err < 0)
 		goto out;
 
-	err = mpls_route_del(cfg);
+	err = mpls_route_del(cfg, extack);
 out:
 	kfree(cfg);
 
@@ -1808,11 +1866,11 @@ static int mpls_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (!cfg)
 		return -ENOMEM;
 
-	err = rtm_to_route_config(skb, nlh, cfg);
+	err = rtm_to_route_config(skb, nlh, cfg, extack);
 	if (err < 0)
 		goto out;
 
-	err = mpls_route_add(cfg);
+	err = mpls_route_add(cfg, extack);
 out:
 	kfree(cfg);
 
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index e59f299ceb3f..a015a6a1143b 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -205,7 +205,7 @@ int nla_put_labels(struct sk_buff *skb, int attrtype,  u8 labels,
 int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
 		   u32 label[], struct netlink_ext_ack *extack);
 int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
-		u8 via[]);
+		u8 via[], struct netlink_ext_ack *extack);
 bool mpls_output_possible(const struct net_device *dev);
 unsigned int mpls_dev_mtu(const struct net_device *dev);
 bool mpls_pkt_too_big(const struct sk_buff *skb, unsigned int mtu);
-- 
2.11.0 (Apple Git-81)

^ permalink raw reply related

* [PATCH net-next 4/6] net: Fill in extack for mpls lwt encap
From: David Ahern @ 2017-05-25  3:54 UTC (permalink / raw)
  To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>

Fill in extack for errors in build_state for mpls lwt encap including
passing extack to nla_get_labels and adding error messages for failures
in it.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/mpls/af_mpls.c       | 49 ++++++++++++++++++++++++++++++++++++++----------
 net/mpls/internal.h      |  2 +-
 net/mpls/mpls_iptunnel.c | 12 +++++++-----
 3 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 257ec66009da..f3830951fb1c 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -728,8 +728,8 @@ static int mpls_nh_build(struct net *net, struct mpls_route *rt,
 		goto errout;
 
 	if (newdst) {
-		err = nla_get_labels(newdst, max_labels,
-				     &nh->nh_labels, nh->nh_label);
+		err = nla_get_labels(newdst, max_labels, &nh->nh_labels,
+				     nh->nh_label, NULL);
 		if (err)
 			goto errout;
 	}
@@ -782,7 +782,8 @@ static u8 mpls_count_nexthops(struct rtnexthop *rtnh, int len,
 
 		nla = nla_find(attrs, attrlen, RTA_NEWDST);
 		if (nla &&
-		    nla_get_labels(nla, MAX_NEW_LABELS, &n_labels, NULL) != 0)
+		    nla_get_labels(nla, MAX_NEW_LABELS, &n_labels,
+				   NULL, NULL) != 0)
 			return 0;
 
 		*max_labels = max_t(u8, *max_labels, n_labels);
@@ -1541,8 +1542,8 @@ int nla_put_labels(struct sk_buff *skb, int attrtype,
 }
 EXPORT_SYMBOL_GPL(nla_put_labels);
 
-int nla_get_labels(const struct nlattr *nla,
-		   u8 max_labels, u8 *labels, u32 label[])
+int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
+		   u32 label[], struct netlink_ext_ack *extack)
 {
 	unsigned len = nla_len(nla);
 	struct mpls_shim_hdr *nla_label;
@@ -1553,13 +1554,18 @@ int nla_get_labels(const struct nlattr *nla,
 	/* len needs to be an even multiple of 4 (the label size). Number
 	 * of labels is a u8 so check for overflow.
 	 */
-	if (len & 3 || len / 4 > 255)
+	if (len & 3 || len / 4 > 255) {
+		NL_SET_ERR_MSG_ATTR(extack, nla,
+				    "Invalid length for labels attribute");
 		return -EINVAL;
+	}
 
 	/* Limit the number of new labels allowed */
 	nla_labels = len/4;
-	if (nla_labels > max_labels)
+	if (nla_labels > max_labels) {
+		NL_SET_ERR_MSG(extack, "Too many labels");
 		return -EINVAL;
+	}
 
 	/* when label == NULL, caller wants number of labels */
 	if (!label)
@@ -1574,8 +1580,29 @@ int nla_get_labels(const struct nlattr *nla,
 		/* Ensure the bottom of stack flag is properly set
 		 * and ttl and tc are both clear.
 		 */
-		if ((dec.bos != bos) || dec.ttl || dec.tc)
+		if (dec.ttl) {
+			NL_SET_ERR_MSG_ATTR(extack, nla,
+					    "TTL in label must be 0");
+			return -EINVAL;
+		}
+
+		if (dec.tc) {
+			NL_SET_ERR_MSG_ATTR(extack, nla,
+					    "Traffic class in label must be 0");
 			return -EINVAL;
+		}
+
+		if (dec.bos != bos) {
+			NL_SET_BAD_ATTR(extack, nla);
+			if (bos) {
+				NL_SET_ERR_MSG(extack,
+					       "BOS bit must be set in first label");
+			} else {
+				NL_SET_ERR_MSG(extack,
+					       "BOS bit can only be set in first label");
+			}
+			return -EINVAL;
+		}
 
 		switch (dec.label) {
 		case MPLS_LABEL_IMPLNULL:
@@ -1583,6 +1610,8 @@ int nla_get_labels(const struct nlattr *nla,
 			 * assign and distribute, but which never
 			 * actually appears in the encapsulation.
 			 */
+			NL_SET_ERR_MSG_ATTR(extack, nla,
+					    "Implicit NULL Label (3) can not be used in encapsulation");
 			return -EINVAL;
 		}
 
@@ -1696,14 +1725,14 @@ static int rtm_to_route_config(struct sk_buff *skb,  struct nlmsghdr *nlh,
 		case RTA_NEWDST:
 			if (nla_get_labels(nla, MAX_NEW_LABELS,
 					   &cfg->rc_output_labels,
-					   cfg->rc_output_label))
+					   cfg->rc_output_label, NULL))
 				goto errout;
 			break;
 		case RTA_DST:
 		{
 			u8 label_count;
 			if (nla_get_labels(nla, 1, &label_count,
-					   &cfg->rc_label))
+					   &cfg->rc_label, NULL))
 				goto errout;
 
 			/* Reserved labels may not be set */
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index 4db6a5971322..e59f299ceb3f 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -203,7 +203,7 @@ static inline struct mpls_dev *mpls_dev_get(const struct net_device *dev)
 int nla_put_labels(struct sk_buff *skb, int attrtype,  u8 labels,
 		   const u32 label[]);
 int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
-		   u32 label[]);
+		   u32 label[], struct netlink_ext_ack *extack);
 int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
 		u8 via[]);
 bool mpls_output_possible(const struct net_device *dev);
diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index 15e1aa708e50..6e558a419f60 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -173,13 +173,14 @@ static int mpls_build_state(struct nlattr *nla,
 	if (ret < 0)
 		return ret;
 
-	if (!tb[MPLS_IPTUNNEL_DST])
+	if (!tb[MPLS_IPTUNNEL_DST]) {
+		NL_SET_ERR_MSG(extack, "MPLS_IPTUNNEL_DST attribute is missing");
 		return -EINVAL;
-
+	}
 
 	/* determine number of labels */
-	if (nla_get_labels(tb[MPLS_IPTUNNEL_DST],
-			   MAX_NEW_LABELS, &n_labels, NULL))
+	if (nla_get_labels(tb[MPLS_IPTUNNEL_DST], MAX_NEW_LABELS,
+			   &n_labels, NULL, extack))
 		return -EINVAL;
 
 	newts = lwtunnel_state_alloc(sizeof(*tun_encap_info) +
@@ -189,7 +190,8 @@ static int mpls_build_state(struct nlattr *nla,
 
 	tun_encap_info = mpls_lwtunnel_encap(newts);
 	ret = nla_get_labels(tb[MPLS_IPTUNNEL_DST], n_labels,
-			     &tun_encap_info->labels, tun_encap_info->label);
+			     &tun_encap_info->labels, tun_encap_info->label,
+			     extack);
 	if (ret)
 		goto errout;
 
-- 
2.11.0 (Apple Git-81)

^ permalink raw reply related


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