lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20180329015007.19496-3-jakub.kicinski@netronome.com>
Date:   Wed, 28 Mar 2018 18:50:07 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     davem@...emloft.net
Cc:     netdev@...r.kernel.org, oss-drivers@...ronome.com,
        John Hurley <john.hurley@...ronome.com>
Subject: [PATCH net-next 2/2] nfp: flower: offload phys port MTU change

From: John Hurley <john.hurley@...ronome.com>

Trigger a port mod message to request an MTU change on the NIC when any
physical port representor is assigned a new MTU value. The driver waits
10 msec for an ack that the FW has set the MTU. If no ack is received the
request is rejected and an appropriate warning flagged.

Rather than maintain an MTU queue per repr, one is maintained per app.
Because the MTU ndo is protected by the rtnl lock, there can never be
contention here. Portmod messages from the NIC are also protected by
rtnl so we first check if the portmod is an ack and, if so, handle outside
rtnl and the cmsg work queue.

Acks are detected by the marking of a bit in a portmod response. They are
then verfied by checking the port number and MTU value expected by the
app. If the expected MTU is 0 then no acks are currently expected.

Also, ensure that the packet headroom reserved by the flower firmware is
considered when accepting an MTU change on any repr.

Signed-off-by: John Hurley <john.hurley@...ronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/cmsg.c | 41 ++++++++++-
 drivers/net/ethernet/netronome/nfp/flower/cmsg.h |  4 +-
 drivers/net/ethernet/netronome/nfp/flower/main.c | 87 +++++++++++++++++++++++-
 drivers/net/ethernet/netronome/nfp/flower/main.h | 19 ++++++
 4 files changed, 146 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c
index baaea6f1a9d8..3735c09d2112 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c
@@ -104,7 +104,8 @@ nfp_flower_cmsg_mac_repr_add(struct sk_buff *skb, unsigned int idx,
 	msg->ports[idx].phys_port = phys_port;
 }
 
-int nfp_flower_cmsg_portmod(struct nfp_repr *repr, bool carrier_ok)
+int nfp_flower_cmsg_portmod(struct nfp_repr *repr, bool carrier_ok,
+			    unsigned int mtu, bool mtu_only)
 {
 	struct nfp_flower_cmsg_portmod *msg;
 	struct sk_buff *skb;
@@ -118,7 +119,11 @@ int nfp_flower_cmsg_portmod(struct nfp_repr *repr, bool carrier_ok)
 	msg->portnum = cpu_to_be32(repr->dst->u.port_info.port_id);
 	msg->reserved = 0;
 	msg->info = carrier_ok;
-	msg->mtu = cpu_to_be16(repr->netdev->mtu);
+
+	if (mtu_only)
+		msg->info |= NFP_FLOWER_CMSG_PORTMOD_MTU_CHANGE_ONLY;
+
+	msg->mtu = cpu_to_be16(mtu);
 
 	nfp_ctrl_tx(repr->app->ctrl, skb);
 
@@ -146,6 +151,34 @@ int nfp_flower_cmsg_portreify(struct nfp_repr *repr, bool exists)
 	return 0;
 }
 
+static bool
+nfp_flower_process_mtu_ack(struct nfp_app *app, struct sk_buff *skb)
+{
+	struct nfp_flower_priv *app_priv = app->priv;
+	struct nfp_flower_cmsg_portmod *msg;
+
+	msg = nfp_flower_cmsg_get_data(skb);
+
+	if (!(msg->info & NFP_FLOWER_CMSG_PORTMOD_MTU_CHANGE_ONLY))
+		return false;
+
+	spin_lock_bh(&app_priv->mtu_conf.lock);
+	if (!app_priv->mtu_conf.requested_val ||
+	    app_priv->mtu_conf.portnum != be32_to_cpu(msg->portnum) ||
+	    be16_to_cpu(msg->mtu) != app_priv->mtu_conf.requested_val) {
+		/* Not an ack for requested MTU change. */
+		spin_unlock_bh(&app_priv->mtu_conf.lock);
+		return false;
+	}
+
+	app_priv->mtu_conf.ack = true;
+	app_priv->mtu_conf.requested_val = 0;
+	wake_up(&app_priv->mtu_conf.wait_q);
+	spin_unlock_bh(&app_priv->mtu_conf.lock);
+
+	return true;
+}
+
 static void
 nfp_flower_cmsg_portmod_rx(struct nfp_app *app, struct sk_buff *skb)
 {
@@ -269,6 +302,10 @@ void nfp_flower_cmsg_rx(struct nfp_app *app, struct sk_buff *skb)
 		/* We need to deal with stats updates from HW asap */
 		nfp_flower_rx_flow_stats(app, skb);
 		dev_consume_skb_any(skb);
+	} else if (cmsg_hdr->type == NFP_FLOWER_CMSG_TYPE_PORT_MOD &&
+		   nfp_flower_process_mtu_ack(app, skb)) {
+		/* Handle MTU acks outside wq to prevent RTNL conflict. */
+		dev_consume_skb_any(skb);
 	} else {
 		skb_queue_tail(&priv->cmsg_skbs, skb);
 		schedule_work(&priv->cmsg_work);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
index 3f46d836d1b8..96bc0e33980c 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
@@ -397,6 +397,7 @@ struct nfp_flower_cmsg_portmod {
 };
 
 #define NFP_FLOWER_CMSG_PORTMOD_INFO_LINK	BIT(0)
+#define NFP_FLOWER_CMSG_PORTMOD_MTU_CHANGE_ONLY	BIT(1)
 
 /* NFP_FLOWER_CMSG_TYPE_PORT_REIFY */
 struct nfp_flower_cmsg_portreify {
@@ -464,7 +465,8 @@ void
 nfp_flower_cmsg_mac_repr_add(struct sk_buff *skb, unsigned int idx,
 			     unsigned int nbi, unsigned int nbi_port,
 			     unsigned int phys_port);
-int nfp_flower_cmsg_portmod(struct nfp_repr *repr, bool carrier_ok);
+int nfp_flower_cmsg_portmod(struct nfp_repr *repr, bool carrier_ok,
+			    unsigned int mtu, bool mtu_only);
 int nfp_flower_cmsg_portreify(struct nfp_repr *repr, bool exists);
 void nfp_flower_cmsg_process_rx(struct work_struct *work);
 void nfp_flower_cmsg_rx(struct nfp_app *app, struct sk_buff *skb);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index 742d6f1575b5..6357e0720f43 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -52,6 +52,8 @@
 
 #define NFP_FLOWER_ALLOWED_VER 0x0001000000010000UL
 
+#define NFP_FLOWER_FRAME_HEADROOM	158
+
 static const char *nfp_flower_extra_cap(struct nfp_app *app, struct nfp_net *nn)
 {
 	return "FLOWER";
@@ -157,7 +159,7 @@ nfp_flower_repr_netdev_open(struct nfp_app *app, struct nfp_repr *repr)
 {
 	int err;
 
-	err = nfp_flower_cmsg_portmod(repr, true);
+	err = nfp_flower_cmsg_portmod(repr, true, repr->netdev->mtu, false);
 	if (err)
 		return err;
 
@@ -171,7 +173,7 @@ nfp_flower_repr_netdev_stop(struct nfp_app *app, struct nfp_repr *repr)
 {
 	netif_tx_disable(repr->netdev);
 
-	return nfp_flower_cmsg_portmod(repr, false);
+	return nfp_flower_cmsg_portmod(repr, false, repr->netdev->mtu, false);
 }
 
 static int
@@ -521,6 +523,9 @@ static int nfp_flower_init(struct nfp_app *app)
 	INIT_WORK(&app_priv->cmsg_work, nfp_flower_cmsg_process_rx);
 	init_waitqueue_head(&app_priv->reify_wait_queue);
 
+	init_waitqueue_head(&app_priv->mtu_conf.wait_q);
+	spin_lock_init(&app_priv->mtu_conf.lock);
+
 	err = nfp_flower_metadata_init(app);
 	if (err)
 		goto err_free_app_priv;
@@ -552,6 +557,81 @@ static void nfp_flower_clean(struct nfp_app *app)
 	app->priv = NULL;
 }
 
+static int
+nfp_flower_check_mtu(struct nfp_app *app, struct net_device *netdev,
+		     int new_mtu)
+{
+	/* The flower fw reserves NFP_FLOWER_FRAME_HEADROOM bytes of the
+	 * supported max MTU to allow for appending tunnel headers. To prevent
+	 * unexpected behaviour this needs to be accounted for.
+	 */
+	if (new_mtu > netdev->max_mtu - NFP_FLOWER_FRAME_HEADROOM) {
+		nfp_err(app->cpp, "New MTU (%d) is not valid\n", new_mtu);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static bool nfp_flower_check_ack(struct nfp_flower_priv *app_priv)
+{
+	bool ret;
+
+	spin_lock_bh(&app_priv->mtu_conf.lock);
+	ret = app_priv->mtu_conf.ack;
+	spin_unlock_bh(&app_priv->mtu_conf.lock);
+
+	return ret;
+}
+
+static int
+nfp_flower_repr_change_mtu(struct nfp_app *app, struct net_device *netdev,
+			   int new_mtu)
+{
+	struct nfp_flower_priv *app_priv = app->priv;
+	struct nfp_repr *repr = netdev_priv(netdev);
+	int err, ack;
+
+	/* Only need to config FW for physical port MTU change. */
+	if (repr->port->type != NFP_PORT_PHYS_PORT)
+		return 0;
+
+	if (!(app_priv->flower_ext_feats & NFP_FL_NBI_MTU_SETTING)) {
+		nfp_err(app->cpp, "Physical port MTU setting not supported\n");
+		return -EINVAL;
+	}
+
+	spin_lock_bh(&app_priv->mtu_conf.lock);
+	app_priv->mtu_conf.ack = false;
+	app_priv->mtu_conf.requested_val = new_mtu;
+	app_priv->mtu_conf.portnum = repr->dst->u.port_info.port_id;
+	spin_unlock_bh(&app_priv->mtu_conf.lock);
+
+	err = nfp_flower_cmsg_portmod(repr, netif_carrier_ok(netdev), new_mtu,
+				      true);
+	if (err) {
+		spin_lock_bh(&app_priv->mtu_conf.lock);
+		app_priv->mtu_conf.requested_val = 0;
+		spin_unlock_bh(&app_priv->mtu_conf.lock);
+		return err;
+	}
+
+	/* Wait for fw to ack the change. */
+	ack = wait_event_timeout(app_priv->mtu_conf.wait_q,
+				 nfp_flower_check_ack(app_priv),
+				 msecs_to_jiffies(10));
+
+	if (!ack) {
+		spin_lock_bh(&app_priv->mtu_conf.lock);
+		app_priv->mtu_conf.requested_val = 0;
+		spin_unlock_bh(&app_priv->mtu_conf.lock);
+		nfp_warn(app->cpp, "MTU change not verified with fw\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int nfp_flower_start(struct nfp_app *app)
 {
 	return nfp_tunnel_config_start(app);
@@ -574,6 +654,9 @@ const struct nfp_app_type app_flower = {
 	.init		= nfp_flower_init,
 	.clean		= nfp_flower_clean,
 
+	.check_mtu	= nfp_flower_check_mtu,
+	.repr_change_mtu  = nfp_flower_repr_change_mtu,
+
 	.vnic_alloc	= nfp_flower_vnic_alloc,
 	.vnic_init	= nfp_flower_vnic_init,
 	.vnic_clean	= nfp_flower_vnic_clean,
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index c5cebf6fb1d3..e030b3ce4510 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -65,6 +65,7 @@ struct nfp_app;
 
 /* Extra features bitmap. */
 #define NFP_FL_FEATS_GENEVE		BIT(0)
+#define NFP_FL_NBI_MTU_SETTING		BIT(1)
 
 struct nfp_fl_mask_id {
 	struct circ_buf mask_id_free_list;
@@ -78,6 +79,22 @@ struct nfp_fl_stats_id {
 	u8 repeated_em_count;
 };
 
+/**
+ * struct nfp_mtu_conf - manage MTU setting
+ * @portnum:		NFP port number of repr with requested MTU change
+ * @requested_val:	MTU value requested for repr
+ * @ack:		Received ack that MTU has been correctly set
+ * @wait_q:		Wait queue for MTU acknowledgements
+ * @lock:		Lock for setting/reading MTU variables
+ */
+struct nfp_mtu_conf {
+	u32 portnum;
+	unsigned int requested_val;
+	bool ack;
+	wait_queue_head_t wait_q;
+	spinlock_t lock;
+};
+
 /**
  * struct nfp_flower_priv - Flower APP per-vNIC priv data
  * @app:		Back pointer to app
@@ -106,6 +123,7 @@ struct nfp_fl_stats_id {
  * @reify_replies:	atomically stores the number of replies received
  *			from firmware for repr reify
  * @reify_wait_queue:	wait queue for repr reify response counting
+ * @mtu_conf:		Configuration of repr MTU value
  */
 struct nfp_flower_priv {
 	struct nfp_app *app;
@@ -133,6 +151,7 @@ struct nfp_flower_priv {
 	struct notifier_block nfp_tun_neigh_nb;
 	atomic_t reify_replies;
 	wait_queue_head_t reify_wait_queue;
+	struct nfp_mtu_conf mtu_conf;
 };
 
 struct nfp_fl_key_ls {
-- 
2.16.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ