On 11/06/2014 22:22, Alex Estrin wrote:
With reference to commit c2904141696ee19551f1553944446f23cdd5d95e
(IPoIB: Fix pkey change flow for virtualization environments)
It was noticed that parent interface keeps sending broadcast group
join requests if p_key index 0 is invalid. That creates unnecessary
noise on a fabric:
ib0: multicast join failed for ff12:401b:8000:0000:0000:0000:ffff:ffff,
status -22
Proposed patch re-init resources, then brings interface
up only if p_key idx 0 is valid either on bootup or on PKEY_CHANGE event.
Original code could run multicast task regardless of p_key value,
which we want to avoid.
Reviewed-by: Ira Weiny <[email protected]>
Signed-off-by: Alex Estrin <[email protected]>
Hi Alex, I re-wrote the change-log to make it clearer and also follow some
practices we use in the kernel (mentioned them to you over prev posts,
but maybe
it wasn't clear enough...) also see some comments further down the patch --
IPoIB: Avoid multicast join attempts when having invalid p_key
Currently, the parent interface keeps sending broadcast group join
requests even if p_key index 0 is invalid, which for itself is
possible/common in virtualized environmentswhere a VF has been probed to
VM but the actual p_key configuration has not yet been assigned by the
management software. This creates unnecessary noise on the fabric and in
the kernel logs:
ib0: multicast join failed for ff12:401b:8000:0000:0000:0000:ffff:ffff,
status -22
The original code run the multicast task regardless of the actual
p_key value, which can be avoided. The fix is to re-init resources and
bring interface up only if p_key index 0 is valid either when starting
up or on PKEY_CHANGE event.
Fixes: c290414169 ('IPoIB: Fix pkey change flow for virtualization
environments')
---
drivers/infiniband/ulp/ipoib/ipoib_ib.c | 35 +++++++++++++++++++------------
1 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index 6a7003d..a2af996 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -52,6 +52,7 @@ MODULE_PARM_DESC(data_debug_level,
#endif
static DEFINE_MUTEX(pkey_mutex);
+static void ipoib_pkey_dev_check_presence(struct net_device *dev);
struct ipoib_ah *ipoib_create_ah(struct net_device *dev,
struct ib_pd *pd, struct ib_ah_attr *attr)
@@ -669,12 +670,13 @@ int ipoib_ib_dev_open(struct net_device *dev)
struct ipoib_dev_priv *priv = netdev_priv(dev);
int ret;
- if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &priv->pkey_index)) {
- ipoib_warn(priv, "P_Key 0x%04x not found\n", priv->pkey);
- clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
+ ipoib_pkey_dev_check_presence(dev);
+
+ if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) {
+ ipoib_warn(priv, "P_Key 0x%04x is %s\n", priv->pkey,
+ !(priv->pkey & 0x7fff) ? "Invalid" : "not found");
CHECK: Alignment should match open parenthesis
#44: FILE: drivers/infiniband/ulp/ipoib/ipoib_ib.c:677:
+ ipoib_warn(priv, "P_Key 0x%04x is %s\n", priv->pkey,
+ !(priv->pkey & 0x7fff) ? "Invalid" : "not found");
please run checkpatch --strict on your patch to fix this one and it's
such (there are more) basically the guideline (based on a quote ofDave
Miller ...) is:
when you have a multi-line function call or if () conditional,the first
non-whitespace character on the second and subsequent lines should line
up with the first column after the openning parenthesis on the first line.
@@ -1038,8 +1043,10 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv
*priv,
ipoib_ib_dev_down(dev, 0);
if (level == IPOIB_FLUSH_HEAVY) {
- ipoib_ib_dev_stop(dev, 0);
- ipoib_ib_dev_open(dev);
+ if (test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags))
+ ipoib_ib_dev_stop(dev, 0);
+ if (ipoib_ib_dev_open(dev) != 0)
+ return;
}
is testing the return code of ipoib_ib_dev_open() really part of the
functional/fix change introduced by this patch or a different fix? if
the latter, put to different/future patch
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
