[PATCH for-next 0/9] mlx4 changes in virtual GID management

[PATCH for-next 0/9] mlx4 changes in virtual GID management

[Or Gerlitz]

Under the existing implementation for virtual GIDs, if the SM is not 
reachable or incurs a delayed response, or if the VF is probed into a
VM before their GUID is registered with the SM, there exists a window 
in time in which the VF sees an incorrect GID, i.e., not the GID that 
was intended by the admin. This results in exposing a temporal identity 
to the VF.

Moreover, a subsequent change in the alias GID causes a spec-incompliant
change to the VF identity. Some guest operating systems, such as Windows,
cannot tolerate such changes.

This series solves above problem by exposing the admin desired value instead 
of the value that was approved by the SM. As long as the SM doesn't approve 
the GID, the VF would see its link as down.

In addition, we request GIDs from the SM on demand, i.e., when a VF actually
needs them, and release them when the GIDs are no longer in use. In cloud
environments, this is useful for GID migrations, in which a GID is assigned to
a VF on the destination HCA, while the VF on the source HCA is shut down (but
the GID was not administratively released).

For reasons of compatibility, an explicit admin request to set/change a GUID
entry is done immediately, regardless of whether the VF is active or not. This
allows administrators to change the GUID without the need to unbind/bind the VF.

In addition, the existing implementation doesn't support a persistency
mechanism to retry a GUID request when the SM has rejected it for any reason.
The PF driver shall keep trying to acquire the specified GUID indefinitely by
utilizing an exponential back off scheme, this should be managed per GUID and
be aligned with other incoming admin requests.

This ability needed especially for the on-demand GUID feature. In this case, we
must manage the GUID's status per entry and handle cases that some entries are
temporarily rejected.

The first patch adds the persistency support and is pre-requisites for the
series.  Further patches make the change to use the admin VF behavior as
described above.

Finally, the default mode is changed to be HOST assigned instead of SM
assigned. This is the expected operational mode, because it doesn't depend on
SM availability as described above.

Yishai and Or.

Yishai Hadas (9):
  IB/mlx4: Alias GUID adding persistency support
  net/mlx4_core: Manage alias GUID per VF
  net/mlx4_core: Set initial admin GUIDs for VFs
  IB/mlx4: Manage admin alias GUID upon admin request
  IB/mlx4: Change init flow to request alias GUIDs for active VFs
  IB/mlx4: Request alias GUID on demand
  net/mlx4_core: Raise slave shutdown event upon FLR
  net/mlx4_core: Return the admin alias GUID upon host view request
  IB/mlx4: Change alias guids default to be host assigned

 drivers/infiniband/hw/mlx4/alias_GUID.c   |  468 +++++++++++++++++++++--------
 drivers/infiniband/hw/mlx4/main.c         |   26 ++-
 drivers/infiniband/hw/mlx4/mlx4_ib.h      |   14 +-
 drivers/infiniband/hw/mlx4/sysfs.c        |   44 +--
 drivers/net/ethernet/mellanox/mlx4/cmd.c  |   42 ++-
 drivers/net/ethernet/mellanox/mlx4/eq.c   |    2 +
 drivers/net/ethernet/mellanox/mlx4/main.c |   39 +++
 drivers/net/ethernet/mellanox/mlx4/mlx4.h |    1 +
 include/linux/mlx4/device.h               |    4 +
 9 files changed, 459 insertions(+), 181 deletions(-)

--
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

[PATCH for-next 1/9] IB/mlx4: Alias GUID adding persistency support

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

If the SM rejects an alias GUID request the PF driver keeps trying to acquire
the specified GUID indefinitely, utilizing an exponential backoff scheme.

Retrying is managed per GUID entry. Each entry that wasn't applied holds its
next retry information. Retry requests to the SM consist of records of 8
consecutive GUIDS. Each record that contains GUIDs requiring retries holds its
next time-to-run based on the retry information of all its GUID entries. The
record having the lowest retry time will run first when that retry time
arrives.

Since the method (SET or DELETE) as sent to the SM applies to all the GUIDs in
the record, we must handle SET requests and DELETE requests in separate SM
messages (one for SETs and the other for DELETEs).

To avoid race conditions where a GUID entry request (set or delete) was
modified after the SM request was sent, we save the method and the requested
indices as part of the callback's context -- thus, only the requested indexes
are evaluated when the response is received.

When an GUID entry is approved we turn off its retry-required bit, this
prevents redundant SM retries from occurring on that record.

The port down event should be sent only when previously it was up. Likewise,
the port up event should be sent only if previously the port was down.

Synchronization was added around the flows that change entries and record state
to prevent race conditions.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/alias_GUID.c |  325 +++++++++++++++++++++++--------
 drivers/infiniband/hw/mlx4/mlx4_ib.h    |    4 +-
 drivers/infiniband/hw/mlx4/sysfs.c      |   11 +-
 3 files changed, 253 insertions(+), 87 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/alias_GUID.c b/drivers/infiniband/hw/mlx4/alias_GUID.c
index a31e031..2ca8984 100644
--- a/drivers/infiniband/hw/mlx4/alias_GUID.c
+++ b/drivers/infiniband/hw/mlx4/alias_GUID.c
@@ -58,14 +58,19 @@ struct mlx4_alias_guid_work_context {
 	int			query_id;
 	struct list_head	list;
 	int			block_num;
+	ib_sa_comp_mask		guid_indexes;
+	u8			method;
 };
 
 struct mlx4_next_alias_guid_work {
 	u8 port;
 	u8 block_num;
+	u8 method;
 	struct mlx4_sriov_alias_guid_info_rec_det rec_det;
 };
 
+static int get_low_record_time_index(struct mlx4_ib_dev *dev, u8 port,
+				     int *resched_delay_sec);
 
 void mlx4_ib_update_cache_on_guid_change(struct mlx4_ib_dev *dev, int block_num,
 					 u8 port_num, u8 *p_data)
@@ -138,10 +143,15 @@ void mlx4_ib_notify_slaves_on_guid_change(struct mlx4_ib_dev *dev,
 	enum slave_port_state prev_state;
 	__be64 tmp_cur_ag, form_cache_ag;
 	enum slave_port_gen_event gen_event;
+	struct mlx4_sriov_alias_guid_info_rec_det *rec;
+	unsigned long flags;
+	__be64 required_value;
 
 	if (!mlx4_is_master(dev->dev))
 		return;
 
+	rec = &dev->sriov.alias_guid.ports_guid[port_num - 1].
+			all_rec_per_port[block_num];
 	guid_indexes = be64_to_cpu((__force __be64) dev->sriov.alias_guid.
 				   ports_guid[port_num - 1].
 				   all_rec_per_port[block_num].guid_indexes);
@@ -166,8 +176,27 @@ void mlx4_ib_notify_slaves_on_guid_change(struct mlx4_ib_dev *dev,
 		 */
 		if (tmp_cur_ag != form_cache_ag)
 			continue;
-		mlx4_gen_guid_change_eqe(dev->dev, slave_id, port_num);
 
+		spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags);
+		required_value = *(__be64 *)&rec->all_recs[i * GUID_REC_SIZE];
+
+		if (required_value == cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL))
+			required_value = 0;
+
+		if (tmp_cur_ag == required_value) {
+			rec->guid_indexes = rec->guid_indexes &
+			       ~mlx4_ib_get_aguid_comp_mask_from_ix(i);
+		} else {
+			/* may notify port down if value is 0 */
+			if (tmp_cur_ag != MLX4_NOT_SET_GUID) {
+				spin_unlock_irqrestore(&dev->sriov.
+					alias_guid.ag_work_lock, flags);
+				continue;
+			}
+		}
+		spin_unlock_irqrestore(&dev->sriov.alias_guid.ag_work_lock,
+				       flags);
+		mlx4_gen_guid_change_eqe(dev->dev, slave_id, port_num);
 		/*2 cases: Valid GUID, and Invalid Guid*/
 
 		if (tmp_cur_ag != MLX4_NOT_SET_GUID) { /*valid GUID*/
@@ -185,13 +214,22 @@ void mlx4_ib_notify_slaves_on_guid_change(struct mlx4_ib_dev *dev,
 							       port_num, MLX4_PORT_CHANGE_SUBTYPE_ACTIVE);
 			}
 		} else { /* request to invalidate GUID */
-			set_and_calc_slave_port_state(dev->dev, slave_id, port_num,
-						      MLX4_PORT_STATE_IB_EVENT_GID_INVALID,
-						      &gen_event);
-			pr_debug("sending PORT DOWN event to slave: %d, port: %d\n",
-				 slave_id, port_num);
-			mlx4_gen_port_state_change_eqe(dev->dev, slave_id, port_num,
-						       MLX4_PORT_CHANGE_SUBTYPE_DOWN);
+			prev_state = mlx4_get_slave_port_state(dev->dev,
+							       slave_id,
+							       port_num);
+			new_state = set_and_calc_slave_port_state(dev->dev,
+								  slave_id,
+								  port_num,
+								  MLX4_PORT_STATE_IB_EVENT_GID_INVALID,
+								  &gen_event);
+			if (gen_event == SLAVE_PORT_GEN_EVENT_DOWN) {
+				pr_debug("sending PORT DOWN event to slave: %d, port: %d\n",
+					 slave_id, port_num);
+				mlx4_gen_port_state_change_eqe(dev->dev,
+							       slave_id,
+							       port_num,
+							       MLX4_PORT_CHANGE_SUBTYPE_DOWN);
+			}
 		}
 	}
 }
@@ -206,6 +244,9 @@ static void aliasguid_query_handler(int status,
 	int i;
 	struct mlx4_sriov_alias_guid_info_rec_det *rec;
 	unsigned long flags, flags1;
+	ib_sa_comp_mask declined_guid_indexes = 0;
+	ib_sa_comp_mask applied_guid_indexes = 0;
+	unsigned int resched_delay_sec = 0;
 
 	if (!context)
 		return;
@@ -216,9 +257,9 @@ static void aliasguid_query_handler(int status,
 		all_rec_per_port[cb_ctx->block_num];
 
 	if (status) {
-		rec->status = MLX4_GUID_INFO_STATUS_IDLE;
 		pr_debug("(port: %d) failed: status = %d\n",
 			 cb_ctx->port, status);
+		rec->time_to_run = ktime_get_real_ns() + 1 * NSEC_PER_SEC;
 		goto out;
 	}
 
@@ -235,57 +276,97 @@ static void aliasguid_query_handler(int status,
 	rec = &dev->sriov.alias_guid.ports_guid[port_index].
 		all_rec_per_port[guid_rec->block_num];
 
-	rec->status = MLX4_GUID_INFO_STATUS_SET;
-	rec->method = MLX4_GUID_INFO_RECORD_SET;
-
+	spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags);
 	for (i = 0 ; i < NUM_ALIAS_GUID_IN_REC; i++) {
-		__be64 tmp_cur_ag;
-		tmp_cur_ag = *(__be64 *)&guid_rec->guid_info_list[i * GUID_REC_SIZE];
+		__be64 sm_response, required_val;
+
+		if (!(cb_ctx->guid_indexes &
+			mlx4_ib_get_aguid_comp_mask_from_ix(i)))
+			continue;
+		sm_response = *(__be64 *)&guid_rec->guid_info_list
+				[i * GUID_REC_SIZE];
+		required_val = *(__be64 *)&rec->all_recs[i * GUID_REC_SIZE];
+		if (cb_ctx->method == MLX4_GUID_INFO_RECORD_DELETE) {
+			if (required_val ==
+			    cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL))
+				goto next_entry;
+
+			/* A new value was set till we got the response */
+			pr_debug("need to set new value %llx, record num %d, block_num:%d\n",
+				 be64_to_cpu(required_val),
+				 i, guid_rec->block_num);
+			goto entry_declined;
+		}
+
 		/* check if the SM didn't assign one of the records.
-		 * if it didn't, if it was not sysadmin request:
-		 * ask the SM to give a new GUID, (instead of the driver request).
+		 * if it didn't, re-ask for.
 		 */
-		if (tmp_cur_ag == MLX4_NOT_SET_GUID) {
-			mlx4_ib_warn(&dev->ib_dev, "%s:Record num %d in "
-				     "block_num: %d was declined by SM, "
-				     "ownership by %d (0 = driver, 1=sysAdmin,"
-				     " 2=None)\n", __func__, i,
-				     guid_rec->block_num, rec->ownership);
-			if (rec->ownership == MLX4_GUID_DRIVER_ASSIGN) {
-				/* if it is driver assign, asks for new GUID from SM*/
-				*(__be64 *)&rec->all_recs[i * GUID_REC_SIZE] =
-					MLX4_NOT_SET_GUID;
-
-				/* Mark the record as not assigned, and let it
-				 * be sent again in the next work sched.*/
-				rec->status = MLX4_GUID_INFO_STATUS_IDLE;
-				rec->guid_indexes |= mlx4_ib_get_aguid_comp_mask_from_ix(i);
-			}
+		if (sm_response == MLX4_NOT_SET_GUID) {
+			if (rec->guids_retry_schedule[i] == 0)
+				mlx4_ib_warn(&dev->ib_dev, "%s:Record num %d in "
+					     "block_num: %d was declined by SM, "
+					     "ownership by %d (0 = driver, 1=sysAdmin,"
+					     " 2=None)\n", __func__, i,
+					     guid_rec->block_num,
+					     rec->ownership);
+			goto entry_declined;
 		} else {
 		       /* properly assigned record. */
 		       /* We save the GUID we just got from the SM in the
 			* admin_guid in order to be persistent, and in the
 			* request from the sm the process will ask for the same GUID */
 			if (rec->ownership == MLX4_GUID_SYSADMIN_ASSIGN &&
-			    tmp_cur_ag != *(__be64 *)&rec->all_recs[i * GUID_REC_SIZE]) {
-				/* the sysadmin assignment failed.*/
-				mlx4_ib_warn(&dev->ib_dev, "%s: Failed to set"
-					     " admin guid after SysAdmin "
-					     "configuration. "
-					     "Record num %d in block_num:%d "
-					     "was declined by SM, "
-					     "new val(0x%llx) was kept\n",
-					      __func__, i,
-					     guid_rec->block_num,
-					     be64_to_cpu(*(__be64 *) &
-							 rec->all_recs[i * GUID_REC_SIZE]));
+			    sm_response != required_val) {
+				/* Warn only on first retry */
+				if (rec->guids_retry_schedule[i] == 0)
+					mlx4_ib_warn(&dev->ib_dev, "%s: Failed to set"
+						     " admin guid after SysAdmin "
+						     "configuration. "
+						     "Record num %d in block_num:%d "
+						     "was declined by SM, "
+						     "new val(0x%llx) was kept, SM returned (0x%llx)\n",
+						      __func__, i,
+						     guid_rec->block_num,
+						     be64_to_cpu(required_val),
+						     be64_to_cpu(sm_response));
+				goto entry_declined;
 			} else {
-				memcpy(&rec->all_recs[i * GUID_REC_SIZE],
-				       &guid_rec->guid_info_list[i * GUID_REC_SIZE],
-				       GUID_REC_SIZE);
+				*(__be64 *)&rec->all_recs[i * GUID_REC_SIZE] =
+					sm_response;
+				goto next_entry;
 			}
 		}
+entry_declined:
+		declined_guid_indexes |= mlx4_ib_get_aguid_comp_mask_from_ix(i);
+		rec->guids_retry_schedule[i] =
+			(rec->guids_retry_schedule[i] == 0) ?  1 :
+			min((unsigned int)60,
+			    rec->guids_retry_schedule[i] * 2);
+		/* using the minimum value among all entries in that record */
+		resched_delay_sec = (resched_delay_sec == 0) ?
+				rec->guids_retry_schedule[i] :
+				min(resched_delay_sec,
+				    rec->guids_retry_schedule[i]);
+		continue;
+
+next_entry:
+		rec->guids_retry_schedule[i] = 0;
 	}
+
+	applied_guid_indexes =  cb_ctx->guid_indexes & ~declined_guid_indexes;
+	if (declined_guid_indexes ||
+	    rec->guid_indexes & ~(applied_guid_indexes)) {
+		pr_debug("record=%d wasn't fully set, guid_indexes=0x%llx applied_indexes=0x%llx, declined_indexes=0x%llx\n",
+			 guid_rec->block_num,
+			 be64_to_cpu((__force __be64)rec->guid_indexes),
+			 be64_to_cpu((__force __be64)applied_guid_indexes),
+			 be64_to_cpu((__force __be64)declined_guid_indexes));
+		rec->time_to_run = ktime_get_real_ns() +
+			resched_delay_sec * NSEC_PER_SEC;
+	} else {
+		rec->status = MLX4_GUID_INFO_STATUS_SET;
+	}
+	spin_unlock_irqrestore(&dev->sriov.alias_guid.ag_work_lock, flags);
 	/*
 	The func is call here to close the cases when the
 	sm doesn't send smp, so in the sa response the driver
@@ -297,10 +378,13 @@ static void aliasguid_query_handler(int status,
 out:
 	spin_lock_irqsave(&dev->sriov.going_down_lock, flags);
 	spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags1);
-	if (!dev->sriov.is_going_down)
+	if (!dev->sriov.is_going_down) {
+		get_low_record_time_index(dev, port_index, &resched_delay_sec);
 		queue_delayed_work(dev->sriov.alias_guid.ports_guid[port_index].wq,
 				   &dev->sriov.alias_guid.ports_guid[port_index].
-				   alias_guid_work, 0);
+				   alias_guid_work,
+				   msecs_to_jiffies(resched_delay_sec * 1000));
+	}
 	if (cb_ctx->sa_query) {
 		list_del(&cb_ctx->list);
 		kfree(cb_ctx);
@@ -317,9 +401,7 @@ static void invalidate_guid_record(struct mlx4_ib_dev *dev, u8 port, int index)
 	ib_sa_comp_mask comp_mask = 0;
 
 	dev->sriov.alias_guid.ports_guid[port - 1].all_rec_per_port[index].status
-		= MLX4_GUID_INFO_STATUS_IDLE;
-	dev->sriov.alias_guid.ports_guid[port - 1].all_rec_per_port[index].method
-		= MLX4_GUID_INFO_RECORD_SET;
+		= MLX4_GUID_INFO_STATUS_SET;
 
 	/* calculate the comp_mask for that record.*/
 	for (i = 0; i < NUM_ALIAS_GUID_IN_REC; i++) {
@@ -340,12 +422,16 @@ static void invalidate_guid_record(struct mlx4_ib_dev *dev, u8 port, int index)
 		comp_mask |= mlx4_ib_get_aguid_comp_mask_from_ix(i);
 	}
 	dev->sriov.alias_guid.ports_guid[port - 1].
-		all_rec_per_port[index].guid_indexes = comp_mask;
+		all_rec_per_port[index].guid_indexes |= comp_mask;
+	if (dev->sriov.alias_guid.ports_guid[port - 1].
+	    all_rec_per_port[index].guid_indexes)
+		dev->sriov.alias_guid.ports_guid[port - 1].
+		all_rec_per_port[index].status = MLX4_GUID_INFO_STATUS_IDLE;
+
 }
 
 static int set_guid_rec(struct ib_device *ibdev,
-			u8 port, int index,
-			struct mlx4_sriov_alias_guid_info_rec_det *rec_det)
+			struct mlx4_next_alias_guid_work *rec)
 {
 	int err;
 	struct mlx4_ib_dev *dev = to_mdev(ibdev);
@@ -354,6 +440,9 @@ static int set_guid_rec(struct ib_device *ibdev,
 	struct ib_port_attr attr;
 	struct mlx4_alias_guid_work_context *callback_context;
 	unsigned long resched_delay, flags, flags1;
+	u8 port = rec->port + 1;
+	int index = rec->block_num;
+	struct mlx4_sriov_alias_guid_info_rec_det *rec_det = &rec->rec_det;
 	struct list_head *head =
 		&dev->sriov.alias_guid.ports_guid[port - 1].cb_list;
 
@@ -380,6 +469,8 @@ static int set_guid_rec(struct ib_device *ibdev,
 	callback_context->port = port;
 	callback_context->dev = dev;
 	callback_context->block_num = index;
+	callback_context->guid_indexes = rec_det->guid_indexes;
+	callback_context->method = rec->method;
 
 	memset(&guid_info_rec, 0, sizeof (struct ib_sa_guidinfo_rec));
 
@@ -399,7 +490,7 @@ static int set_guid_rec(struct ib_device *ibdev,
 	callback_context->query_id =
 		ib_sa_guid_info_rec_query(dev->sriov.alias_guid.sa_client,
 					  ibdev, port, &guid_info_rec,
-					  comp_mask, rec_det->method, 1000,
+					  comp_mask, rec->method, 1000,
 					  GFP_KERNEL, aliasguid_query_handler,
 					  callback_context,
 					  &callback_context->sa_query);
@@ -462,31 +553,107 @@ void mlx4_ib_invalidate_all_guid_record(struct mlx4_ib_dev *dev, int port)
 	spin_unlock_irqrestore(&dev->sriov.going_down_lock, flags);
 }
 
+static void set_required_record(struct mlx4_ib_dev *dev, u8 port,
+				struct mlx4_next_alias_guid_work *next_rec,
+				int record_index)
+{
+	int i;
+	int lowset_time_entry = -1;
+	int lowest_time = 0;
+	ib_sa_comp_mask delete_guid_indexes = 0;
+	ib_sa_comp_mask set_guid_indexes = 0;
+	struct mlx4_sriov_alias_guid_info_rec_det *rec =
+			&dev->sriov.alias_guid.ports_guid[port].
+			all_rec_per_port[record_index];
+
+	for (i = 0; i < NUM_ALIAS_GUID_IN_REC; i++) {
+		if (!(rec->guid_indexes &
+			mlx4_ib_get_aguid_comp_mask_from_ix(i)))
+			continue;
+
+		if (*(__be64 *)&rec->all_recs[i * GUID_REC_SIZE] ==
+				cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL))
+			delete_guid_indexes |=
+				mlx4_ib_get_aguid_comp_mask_from_ix(i);
+		else
+			set_guid_indexes |=
+				mlx4_ib_get_aguid_comp_mask_from_ix(i);
+
+		if (lowset_time_entry == -1 || rec->guids_retry_schedule[i] <=
+			lowest_time) {
+			lowset_time_entry = i;
+			lowest_time = rec->guids_retry_schedule[i];
+		}
+	}
+
+	memcpy(&next_rec->rec_det, rec, sizeof(*rec));
+	next_rec->port = port;
+	next_rec->block_num = record_index;
+
+	if (*(__be64 *)&rec->all_recs[lowset_time_entry * GUID_REC_SIZE] ==
+				cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL)) {
+		next_rec->rec_det.guid_indexes = delete_guid_indexes;
+		next_rec->method = MLX4_GUID_INFO_RECORD_DELETE;
+	} else {
+		next_rec->rec_det.guid_indexes = set_guid_indexes;
+		next_rec->method = MLX4_GUID_INFO_RECORD_SET;
+	}
+}
+
+/* return index of record that should be updated based on lowest
+ * rescheduled time
+ */
+static int get_low_record_time_index(struct mlx4_ib_dev *dev, u8 port,
+				     int *resched_delay_sec)
+{
+	int record_index = -1;
+	u64 low_record_time = 0;
+	struct mlx4_sriov_alias_guid_info_rec_det rec;
+	int j;
+
+	for (j = 0; j < NUM_ALIAS_GUID_REC_IN_PORT; j++) {
+		rec = dev->sriov.alias_guid.ports_guid[port].
+			all_rec_per_port[j];
+		if (rec.status == MLX4_GUID_INFO_STATUS_IDLE &&
+		    rec.guid_indexes) {
+			if (record_index == -1 ||
+			    rec.time_to_run < low_record_time) {
+				record_index = j;
+				low_record_time = rec.time_to_run;
+			}
+		}
+	}
+	if (resched_delay_sec) {
+		u64 curr_time = ktime_get_real_ns();
+
+		*resched_delay_sec = (low_record_time < curr_time) ? 0 :
+			div_u64((low_record_time - curr_time), NSEC_PER_SEC);
+	}
+
+	return record_index;
+}
+
 /* The function returns the next record that was
  * not configured (or failed to be configured) */
 static int get_next_record_to_update(struct mlx4_ib_dev *dev, u8 port,
 				     struct mlx4_next_alias_guid_work *rec)
 {
-	int j;
 	unsigned long flags;
+	int record_index;
+	int ret = 0;
 
-	for (j = 0; j < NUM_ALIAS_GUID_REC_IN_PORT; j++) {
-		spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags);
-		if (dev->sriov.alias_guid.ports_guid[port].all_rec_per_port[j].status ==
-		    MLX4_GUID_INFO_STATUS_IDLE) {
-			memcpy(&rec->rec_det,
-			       &dev->sriov.alias_guid.ports_guid[port].all_rec_per_port[j],
-			       sizeof (struct mlx4_sriov_alias_guid_info_rec_det));
-			rec->port = port;
-			rec->block_num = j;
-			dev->sriov.alias_guid.ports_guid[port].all_rec_per_port[j].status =
-				MLX4_GUID_INFO_STATUS_PENDING;
-			spin_unlock_irqrestore(&dev->sriov.alias_guid.ag_work_lock, flags);
-			return 0;
-		}
-		spin_unlock_irqrestore(&dev->sriov.alias_guid.ag_work_lock, flags);
+	spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags);
+	record_index = get_low_record_time_index(dev, port, NULL);
+
+	if (record_index < 0) {
+		ret = -ENOENT;
+		goto out;
 	}
-	return -ENOENT;
+
+	set_required_record(dev, port, rec, record_index);
+out:
+	spin_unlock_irqrestore(&dev->sriov.alias_guid.ag_work_lock, flags);
+	return ret;
 }
 
 static void set_administratively_guid_record(struct mlx4_ib_dev *dev, int port,
@@ -497,8 +664,6 @@ static void set_administratively_guid_record(struct mlx4_ib_dev *dev, int port,
 		rec_det->guid_indexes;
 	memcpy(dev->sriov.alias_guid.ports_guid[port].all_rec_per_port[rec_index].all_recs,
 	       rec_det->all_recs, NUM_ALIAS_GUID_IN_REC * GUID_REC_SIZE);
-	dev->sriov.alias_guid.ports_guid[port].all_rec_per_port[rec_index].status =
-		rec_det->status;
 }
 
 static void set_all_slaves_guids(struct mlx4_ib_dev *dev, int port)
@@ -545,9 +710,7 @@ static void alias_guid_work(struct work_struct *work)
 		goto out;
 	}
 
-	set_guid_rec(&dev->ib_dev, rec->port + 1, rec->block_num,
-		     &rec->rec_det);
-
+	set_guid_rec(&dev->ib_dev, rec);
 out:
 	kfree(rec);
 }
@@ -562,6 +725,12 @@ void mlx4_ib_init_alias_guid_work(struct mlx4_ib_dev *dev, int port)
 	spin_lock_irqsave(&dev->sriov.going_down_lock, flags);
 	spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags1);
 	if (!dev->sriov.is_going_down) {
+		/* If there is pending one should cancell then run, otherwise
+		  * won't run till previous one is ended as same work
+		  * struct is used.
+		  */
+		cancel_delayed_work(&dev->sriov.alias_guid.ports_guid[port].
+				    alias_guid_work);
 		queue_delayed_work(dev->sriov.alias_guid.ports_guid[port].wq,
 			   &dev->sriov.alias_guid.ports_guid[port].alias_guid_work, 0);
 	}
diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h b/drivers/infiniband/hw/mlx4/mlx4_ib.h
index f829fd9..1cfc2bb 100644
--- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
+++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
@@ -342,7 +342,6 @@ struct mlx4_ib_ah {
 enum mlx4_guid_alias_rec_status {
 	MLX4_GUID_INFO_STATUS_IDLE,
 	MLX4_GUID_INFO_STATUS_SET,
-	MLX4_GUID_INFO_STATUS_PENDING,
 };
 
 enum mlx4_guid_alias_rec_ownership {
@@ -360,8 +359,9 @@ struct mlx4_sriov_alias_guid_info_rec_det {
 	u8 all_recs[GUID_REC_SIZE * NUM_ALIAS_GUID_IN_REC];
 	ib_sa_comp_mask guid_indexes; /*indicates what from the 8 records are valid*/
 	enum mlx4_guid_alias_rec_status status; /*indicates the administraively status of the record.*/
-	u8 method; /*set or delete*/
 	enum mlx4_guid_alias_rec_ownership ownership; /*indicates who assign that alias_guid record*/
+	unsigned int guids_retry_schedule[NUM_ALIAS_GUID_IN_REC];
+	u64 time_to_run;
 };
 
 struct mlx4_sriov_alias_guid_port_rec_det {
diff --git a/drivers/infiniband/hw/mlx4/sysfs.c b/drivers/infiniband/hw/mlx4/sysfs.c
index d10c2b8..7423d7e 100644
--- a/drivers/infiniband/hw/mlx4/sysfs.c
+++ b/drivers/infiniband/hw/mlx4/sysfs.c
@@ -80,6 +80,7 @@ static ssize_t store_admin_alias_guid(struct device *dev,
 	struct mlx4_ib_iov_port *port = mlx4_ib_iov_dentry->ctx;
 	struct mlx4_ib_dev *mdev = port->dev;
 	u64 sysadmin_ag_val;
+	unsigned long flags;
 
 	record_num = mlx4_ib_iov_dentry->entry_num / 8;
 	guid_index_in_rec = mlx4_ib_iov_dentry->entry_num % 8;
@@ -87,6 +88,7 @@ static ssize_t store_admin_alias_guid(struct device *dev,
 		pr_err("GUID 0 block 0 is RO\n");
 		return count;
 	}
+	spin_lock_irqsave(&mdev->sriov.alias_guid.ag_work_lock, flags);
 	sscanf(buf, "%llx", &sysadmin_ag_val);
 	*(__be64 *)&mdev->sriov.alias_guid.ports_guid[port->num - 1].
 		all_rec_per_port[record_num].
@@ -96,14 +98,8 @@ static ssize_t store_admin_alias_guid(struct device *dev,
 	/* Change the state to be pending for update */
 	mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].status
 		= MLX4_GUID_INFO_STATUS_IDLE ;
-
-	mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].method
-		= MLX4_GUID_INFO_RECORD_SET;
-
 	switch (sysadmin_ag_val) {
 	case MLX4_GUID_FOR_DELETE_VAL:
-		mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].method
-			= MLX4_GUID_INFO_RECORD_DELETE;
 		mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].ownership
 			= MLX4_GUID_SYSADMIN_ASSIGN;
 		break;
@@ -121,8 +117,9 @@ static ssize_t store_admin_alias_guid(struct device *dev,
 
 	/* set the record index */
 	mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].guid_indexes
-		= mlx4_ib_get_aguid_comp_mask_from_ix(guid_index_in_rec);
+		|= mlx4_ib_get_aguid_comp_mask_from_ix(guid_index_in_rec);
 
+	spin_unlock_irqrestore(&mdev->sriov.alias_guid.ag_work_lock, flags);
 	mlx4_ib_init_alias_guid_work(mdev, port->num - 1);
 
 	return count;
-- 
1.7.1

--
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

[PATCH for-next 2/9] net/mlx4_core: Manage alias GUID per VF

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Manages alias GUIDs per VF per port in the core layer.

This is a pre-step for managing alias GUIDs in a mode that the admin
GUID is returned via ib_query_gid() regardless of whether the SM
has approved it or not.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx4/main.c |   16 ++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/mlx4.h |    1 +
 include/linux/mlx4/device.h               |    3 +++
 3 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 43aa767..6d1f10e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2229,6 +2229,22 @@ void mlx4_counter_free(struct mlx4_dev *dev, u32 idx)
 }
 EXPORT_SYMBOL_GPL(mlx4_counter_free);
 
+void mlx4_set_admin_guid(struct mlx4_dev *dev, __be64 guid, int entry, int port)
+{
+	struct mlx4_priv *priv = mlx4_priv(dev);
+
+	priv->mfunc.master.vf_admin[entry].vport[port].guid = guid;
+}
+EXPORT_SYMBOL_GPL(mlx4_set_admin_guid);
+
+__be64 mlx4_get_admin_guid(struct mlx4_dev *dev, int entry, int port)
+{
+	struct mlx4_priv *priv = mlx4_priv(dev);
+
+	return priv->mfunc.master.vf_admin[entry].vport[port].guid;
+}
+EXPORT_SYMBOL_GPL(mlx4_get_admin_guid);
+
 static int mlx4_setup_hca(struct mlx4_dev *dev)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index 0b16db0..20165fb 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -512,6 +512,7 @@ struct mlx4_vport_state {
 	u32 tx_rate;
 	bool spoofchk;
 	u32 link_state;
+	__be64 guid;
 };
 
 struct mlx4_vf_admin_state {
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 4550c67..5c67bf0 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -1337,6 +1337,9 @@ int mlx4_wol_write(struct mlx4_dev *dev, u64 config, int port);
 int mlx4_counter_alloc(struct mlx4_dev *dev, u32 *idx);
 void mlx4_counter_free(struct mlx4_dev *dev, u32 idx);
 
+void mlx4_set_admin_guid(struct mlx4_dev *dev, __be64 guid, int entry,
+			 int port);
+__be64 mlx4_get_admin_guid(struct mlx4_dev *dev, int entry, int port);
 int mlx4_flow_attach(struct mlx4_dev *dev,
 		     struct mlx4_net_trans_rule *rule, u64 *reg_id);
 int mlx4_flow_detach(struct mlx4_dev *dev, u64 reg_id);
-- 
1.7.1

--
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

[PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

To have out of the box experience, the PF generates random GUIDs who
serve as the initial admin values.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c  |    1 +
 drivers/net/ethernet/mellanox/mlx4/main.c |   23 +++++++++++++++++++++++
 include/linux/mlx4/device.h               |    1 +
 3 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 20b3c7b..778de74 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -2255,6 +2255,7 @@ int mlx4_multi_func_init(struct mlx4_dev *dev)
 				priv->mfunc.master.vf_oper[i].vport[port].state.default_vlan = MLX4_VGT;
 				priv->mfunc.master.vf_oper[i].vport[port].vlan_idx = NO_INDX;
 				priv->mfunc.master.vf_oper[i].vport[port].mac_idx = NO_INDX;
+				mlx4_set_random_admin_guid(dev, i, port);
 			}
 			spin_lock_init(&s_state->lock);
 		}
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 6d1f10e..67e57e5 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -42,6 +42,7 @@
 #include <linux/io-mapping.h>
 #include <linux/delay.h>
 #include <linux/kmod.h>
+#include <linux/etherdevice.h>
 
 #include <linux/mlx4/device.h>
 #include <linux/mlx4/doorbell.h>
@@ -2245,6 +2246,28 @@ __be64 mlx4_get_admin_guid(struct mlx4_dev *dev, int entry, int port)
 }
 EXPORT_SYMBOL_GPL(mlx4_get_admin_guid);
 
+void mlx4_set_random_admin_guid(struct mlx4_dev *dev, int entry, int port)
+{
+	struct mlx4_priv *priv = mlx4_priv(dev);
+	u8 random_mac[6];
+	char *raw_gid;
+
+	/* hw GUID */
+	if (entry == 0)
+		return;
+
+	eth_random_addr(random_mac);
+	raw_gid = (char *)&priv->mfunc.master.vf_admin[entry].vport[port].guid;
+	raw_gid[0] = random_mac[0] ^ 2;
+	raw_gid[1] = random_mac[1];
+	raw_gid[2] = random_mac[2];
+	raw_gid[3] = 0xff;
+	raw_gid[4] = 0xfe;
+	raw_gid[5] = random_mac[3];
+	raw_gid[6] = random_mac[4];
+	raw_gid[7] = random_mac[5];
+}
+
 static int mlx4_setup_hca(struct mlx4_dev *dev)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 5c67bf0..f867d25 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -1340,6 +1340,7 @@ void mlx4_counter_free(struct mlx4_dev *dev, u32 idx);
 void mlx4_set_admin_guid(struct mlx4_dev *dev, __be64 guid, int entry,
 			 int port);
 __be64 mlx4_get_admin_guid(struct mlx4_dev *dev, int entry, int port);
+void mlx4_set_random_admin_guid(struct mlx4_dev *dev, int entry, int port);
 int mlx4_flow_attach(struct mlx4_dev *dev,
 		     struct mlx4_net_trans_rule *rule, u64 *reg_id);
 int mlx4_flow_detach(struct mlx4_dev *dev, u64 reg_id);
-- 
1.7.1

--
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

[PATCH for-next 4/9] IB/mlx4: Manage admin alias GUID upon admin request

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Set the admin alias GUID per the administrator's request via the sysfs
mechanism into the core layer.

The "get" request returns the current value. However, if the administrator
requests the SM to assign a new value by requesting 0, the SM assigned
GUID is returned.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/alias_GUID.c |    6 ++++++
 drivers/infiniband/hw/mlx4/sysfs.c      |   18 +++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/alias_GUID.c b/drivers/infiniband/hw/mlx4/alias_GUID.c
index 2ca8984..c20ce09 100644
--- a/drivers/infiniband/hw/mlx4/alias_GUID.c
+++ b/drivers/infiniband/hw/mlx4/alias_GUID.c
@@ -333,6 +333,12 @@ static void aliasguid_query_handler(int status,
 			} else {
 				*(__be64 *)&rec->all_recs[i * GUID_REC_SIZE] =
 					sm_response;
+				if (required_val == 0)
+					mlx4_set_admin_guid(dev->dev,
+							    sm_response,
+							    (guid_rec->block_num
+							    * NUM_ALIAS_GUID_IN_REC) + i,
+							    cb_ctx->port);
 				goto next_entry;
 			}
 		}
diff --git a/drivers/infiniband/hw/mlx4/sysfs.c b/drivers/infiniband/hw/mlx4/sysfs.c
index 7423d7e..bb1c34a 100644
--- a/drivers/infiniband/hw/mlx4/sysfs.c
+++ b/drivers/infiniband/hw/mlx4/sysfs.c
@@ -46,21 +46,17 @@
 static ssize_t show_admin_alias_guid(struct device *dev,
 			      struct device_attribute *attr, char *buf)
 {
-	int record_num;/*0-15*/
-	int guid_index_in_rec; /*0 - 7*/
 	struct mlx4_ib_iov_sysfs_attr *mlx4_ib_iov_dentry =
 		container_of(attr, struct mlx4_ib_iov_sysfs_attr, dentry);
 	struct mlx4_ib_iov_port *port = mlx4_ib_iov_dentry->ctx;
 	struct mlx4_ib_dev *mdev = port->dev;
+	__be64 sysadmin_ag_val;
 
-	record_num = mlx4_ib_iov_dentry->entry_num / 8 ;
-	guid_index_in_rec = mlx4_ib_iov_dentry->entry_num % 8 ;
+	sysadmin_ag_val = mlx4_get_admin_guid(mdev->dev,
+					      mlx4_ib_iov_dentry->entry_num,
+					      port->num);
 
-	return sprintf(buf, "%llx\n",
-		       be64_to_cpu(*(__be64 *)&mdev->sriov.alias_guid.
-				   ports_guid[port->num - 1].
-				   all_rec_per_port[record_num].
-				   all_recs[8 * guid_index_in_rec]));
+	return sprintf(buf, "%llx\n", be64_to_cpu(sysadmin_ag_val));
 }
 
 /* store_admin_alias_guid stores the (new) administratively assigned value of that GUID.
@@ -98,6 +94,10 @@ static ssize_t store_admin_alias_guid(struct device *dev,
 	/* Change the state to be pending for update */
 	mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].status
 		= MLX4_GUID_INFO_STATUS_IDLE ;
+	mlx4_set_admin_guid(mdev->dev, cpu_to_be64(sysadmin_ag_val),
+			    mlx4_ib_iov_dentry->entry_num,
+			    port->num);
+
 	switch (sysadmin_ag_val) {
 	case MLX4_GUID_FOR_DELETE_VAL:
 		mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].ownership
-- 
1.7.1

--
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

[PATCH for-next 5/9] IB/mlx4: Change init flow to request alias GUIDs for active VFs

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Change the init flow to ask GUIDs only for active VFs. This is done for
both SM & HOST modes so that there is no need any more to maintain the
ownership record type.

In case SM mode is used, the initial value will be 0, ask the SM to assign,
for the HOST mode the initial value will be the HOST generated GUID.

This will enable out of the box experience for both probed and attached VFs.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/alias_GUID.c |  104 ++++++++++++++-----------------
 drivers/infiniband/hw/mlx4/mlx4_ib.h    |    8 +--
 drivers/infiniband/hw/mlx4/sysfs.c      |   17 -----
 3 files changed, 50 insertions(+), 79 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/alias_GUID.c b/drivers/infiniband/hw/mlx4/alias_GUID.c
index c20ce09..cbadab0 100644
--- a/drivers/infiniband/hw/mlx4/alias_GUID.c
+++ b/drivers/infiniband/hw/mlx4/alias_GUID.c
@@ -303,19 +303,17 @@ static void aliasguid_query_handler(int status,
 		 */
 		if (sm_response == MLX4_NOT_SET_GUID) {
 			if (rec->guids_retry_schedule[i] == 0)
-				mlx4_ib_warn(&dev->ib_dev, "%s:Record num %d in "
-					     "block_num: %d was declined by SM, "
-					     "ownership by %d (0 = driver, 1=sysAdmin,"
-					     " 2=None)\n", __func__, i,
-					     guid_rec->block_num,
-					     rec->ownership);
+				mlx4_ib_warn(&dev->ib_dev,
+					     "%s:Record num %d in  block_num: %d was declined by SM\n",
+					     __func__, i,
+					     guid_rec->block_num);
 			goto entry_declined;
 		} else {
 		       /* properly assigned record. */
 		       /* We save the GUID we just got from the SM in the
 			* admin_guid in order to be persistent, and in the
 			* request from the sm the process will ask for the same GUID */
-			if (rec->ownership == MLX4_GUID_SYSADMIN_ASSIGN &&
+			if (required_val &&
 			    sm_response != required_val) {
 				/* Warn only on first retry */
 				if (rec->guids_retry_schedule[i] == 0)
@@ -421,9 +419,7 @@ static void invalidate_guid_record(struct mlx4_ib_dev *dev, u8 port, int index)
 		need to assign GUIDs, then don't put it up for assignment.
 		*/
 		if (MLX4_GUID_FOR_DELETE_VAL == cur_admin_val ||
-		    (!index && !i) ||
-		    MLX4_GUID_NONE_ASSIGN == dev->sriov.alias_guid.
-		    ports_guid[port - 1].all_rec_per_port[index].ownership)
+		    (!index && !i))
 			continue;
 		comp_mask |= mlx4_ib_get_aguid_comp_mask_from_ix(i);
 	}
@@ -531,6 +527,30 @@ out:
 	return err;
 }
 
+static void mlx4_ib_guid_port_init(struct mlx4_ib_dev *dev, int port)
+{
+	int j, k, entry;
+	__be64 guid;
+
+	/*Check if the SM doesn't need to assign the GUIDs*/
+	for (j = 0; j < NUM_ALIAS_GUID_REC_IN_PORT; j++) {
+		for (k = 0; k < NUM_ALIAS_GUID_IN_REC; k++) {
+			entry = j * NUM_ALIAS_GUID_IN_REC + k;
+			/* no request for the 0 entry (hw guid) */
+			if (!entry || entry > dev->dev->persist->num_vfs ||
+			    !mlx4_is_slave_active(dev->dev, entry))
+				continue;
+			guid = mlx4_get_admin_guid(dev->dev, entry, port);
+			*(__be64 *)&dev->sriov.alias_guid.ports_guid[port - 1].
+				all_rec_per_port[j].all_recs
+				[GUID_REC_SIZE * k] = guid;
+			pr_debug("guid was set, entry=%d, val=0x%llx, port=%d\n",
+				 entry,
+				 be64_to_cpu(guid),
+				 port);
+		}
+	}
+}
 void mlx4_ib_invalidate_all_guid_record(struct mlx4_ib_dev *dev, int port)
 {
 	int i;
@@ -540,6 +560,13 @@ void mlx4_ib_invalidate_all_guid_record(struct mlx4_ib_dev *dev, int port)
 
 	spin_lock_irqsave(&dev->sriov.going_down_lock, flags);
 	spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags1);
+
+	if (dev->sriov.alias_guid.ports_guid[port - 1].state_flags &
+		GUID_STATE_NEED_PORT_INIT) {
+		mlx4_ib_guid_port_init(dev, port);
+		dev->sriov.alias_guid.ports_guid[port - 1].state_flags &=
+			(~GUID_STATE_NEED_PORT_INIT);
+	}
 	for (i = 0; i < NUM_ALIAS_GUID_REC_IN_PORT; i++)
 		invalidate_guid_record(dev, port, i);
 
@@ -662,33 +689,6 @@ out:
 	return ret;
 }
 
-static void set_administratively_guid_record(struct mlx4_ib_dev *dev, int port,
-					     int rec_index,
-					     struct mlx4_sriov_alias_guid_info_rec_det *rec_det)
-{
-	dev->sriov.alias_guid.ports_guid[port].all_rec_per_port[rec_index].guid_indexes =
-		rec_det->guid_indexes;
-	memcpy(dev->sriov.alias_guid.ports_guid[port].all_rec_per_port[rec_index].all_recs,
-	       rec_det->all_recs, NUM_ALIAS_GUID_IN_REC * GUID_REC_SIZE);
-}
-
-static void set_all_slaves_guids(struct mlx4_ib_dev *dev, int port)
-{
-	int j;
-	struct mlx4_sriov_alias_guid_info_rec_det rec_det ;
-
-	for (j = 0 ; j < NUM_ALIAS_GUID_REC_IN_PORT ; j++) {
-		memset(rec_det.all_recs, 0, NUM_ALIAS_GUID_IN_REC * GUID_REC_SIZE);
-		rec_det.guid_indexes = (!j ? 0 : IB_SA_GUIDINFO_REC_GID0) |
-			IB_SA_GUIDINFO_REC_GID1 | IB_SA_GUIDINFO_REC_GID2 |
-			IB_SA_GUIDINFO_REC_GID3 | IB_SA_GUIDINFO_REC_GID4 |
-			IB_SA_GUIDINFO_REC_GID5 | IB_SA_GUIDINFO_REC_GID6 |
-			IB_SA_GUIDINFO_REC_GID7;
-		rec_det.status = MLX4_GUID_INFO_STATUS_IDLE;
-		set_administratively_guid_record(dev, port, j, &rec_det);
-	}
-}
-
 static void alias_guid_work(struct work_struct *work)
 {
 	struct delayed_work *delay = to_delayed_work(work);
@@ -784,7 +784,7 @@ int mlx4_ib_init_alias_guid_service(struct mlx4_ib_dev *dev)
 {
 	char alias_wq_name[15];
 	int ret = 0;
-	int i, j, k;
+	int i, j;
 	union ib_gid gid;
 
 	if (!mlx4_is_master(dev->dev))
@@ -808,33 +808,25 @@ int mlx4_ib_init_alias_guid_service(struct mlx4_ib_dev *dev)
 	for (i = 0 ; i < dev->num_ports; i++) {
 		memset(&dev->sriov.alias_guid.ports_guid[i], 0,
 		       sizeof (struct mlx4_sriov_alias_guid_port_rec_det));
-		/*Check if the SM doesn't need to assign the GUIDs*/
+		dev->sriov.alias_guid.ports_guid[i].state_flags |=
+				GUID_STATE_NEED_PORT_INIT;
 		for (j = 0; j < NUM_ALIAS_GUID_REC_IN_PORT; j++) {
-			if (mlx4_ib_sm_guid_assign) {
-				dev->sriov.alias_guid.ports_guid[i].
-					all_rec_per_port[j].
-					ownership = MLX4_GUID_DRIVER_ASSIGN;
-				continue;
-			}
-			dev->sriov.alias_guid.ports_guid[i].all_rec_per_port[j].
-					ownership = MLX4_GUID_NONE_ASSIGN;
-			/*mark each val as it was deleted,
-			  till the sysAdmin will give it valid val*/
-			for (k = 0; k < NUM_ALIAS_GUID_IN_REC; k++) {
-				*(__be64 *)&dev->sriov.alias_guid.ports_guid[i].
-					all_rec_per_port[j].all_recs[GUID_REC_SIZE * k] =
-						cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL);
-			}
+			/* mark each val as it was deleted */
+			memset(dev->sriov.alias_guid.ports_guid[i].
+				all_rec_per_port[j].all_recs, 0xFF,
+				sizeof(dev->sriov.alias_guid.ports_guid[i].
+				all_rec_per_port[j].all_recs));
 		}
 		INIT_LIST_HEAD(&dev->sriov.alias_guid.ports_guid[i].cb_list);
 		/*prepare the records, set them to be allocated by sm*/
+		if (mlx4_ib_sm_guid_assign)
+			for (j = 1; j < NUM_ALIAS_GUID_PER_PORT; j++)
+				mlx4_set_admin_guid(dev->dev, 0, j, i + 1);
 		for (j = 0 ; j < NUM_ALIAS_GUID_REC_IN_PORT; j++)
 			invalidate_guid_record(dev, i + 1, j);
 
 		dev->sriov.alias_guid.ports_guid[i].parent = &dev->sriov.alias_guid;
 		dev->sriov.alias_guid.ports_guid[i].port  = i;
-		if (mlx4_ib_sm_guid_assign)
-			set_all_slaves_guids(dev, i);
 
 		snprintf(alias_wq_name, sizeof alias_wq_name, "alias_guid%d", i);
 		dev->sriov.alias_guid.ports_guid[i].wq =
diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h b/drivers/infiniband/hw/mlx4/mlx4_ib.h
index 1cfc2bb..532a877 100644
--- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
+++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
@@ -344,11 +344,7 @@ enum mlx4_guid_alias_rec_status {
 	MLX4_GUID_INFO_STATUS_SET,
 };
 
-enum mlx4_guid_alias_rec_ownership {
-	MLX4_GUID_DRIVER_ASSIGN,
-	MLX4_GUID_SYSADMIN_ASSIGN,
-	MLX4_GUID_NONE_ASSIGN, /*init state of each record*/
-};
+#define GUID_STATE_NEED_PORT_INIT 0x01
 
 enum mlx4_guid_alias_rec_method {
 	MLX4_GUID_INFO_RECORD_SET	= IB_MGMT_METHOD_SET,
@@ -359,7 +355,6 @@ struct mlx4_sriov_alias_guid_info_rec_det {
 	u8 all_recs[GUID_REC_SIZE * NUM_ALIAS_GUID_IN_REC];
 	ib_sa_comp_mask guid_indexes; /*indicates what from the 8 records are valid*/
 	enum mlx4_guid_alias_rec_status status; /*indicates the administraively status of the record.*/
-	enum mlx4_guid_alias_rec_ownership ownership; /*indicates who assign that alias_guid record*/
 	unsigned int guids_retry_schedule[NUM_ALIAS_GUID_IN_REC];
 	u64 time_to_run;
 };
@@ -369,6 +364,7 @@ struct mlx4_sriov_alias_guid_port_rec_det {
 	struct workqueue_struct *wq;
 	struct delayed_work alias_guid_work;
 	u8 port;
+	u32 state_flags;
 	struct mlx4_sriov_alias_guid *parent;
 	struct list_head cb_list;
 };
diff --git a/drivers/infiniband/hw/mlx4/sysfs.c b/drivers/infiniband/hw/mlx4/sysfs.c
index bb1c34a..6797108 100644
--- a/drivers/infiniband/hw/mlx4/sysfs.c
+++ b/drivers/infiniband/hw/mlx4/sysfs.c
@@ -98,23 +98,6 @@ static ssize_t store_admin_alias_guid(struct device *dev,
 			    mlx4_ib_iov_dentry->entry_num,
 			    port->num);
 
-	switch (sysadmin_ag_val) {
-	case MLX4_GUID_FOR_DELETE_VAL:
-		mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].ownership
-			= MLX4_GUID_SYSADMIN_ASSIGN;
-		break;
-	/* The sysadmin requests the SM to re-assign */
-	case MLX4_NOT_SET_GUID:
-		mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].ownership
-			= MLX4_GUID_DRIVER_ASSIGN;
-		break;
-	/* The sysadmin requests a specific value.*/
-	default:
-		mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].ownership
-			= MLX4_GUID_SYSADMIN_ASSIGN;
-		break;
-	}
-
 	/* set the record index */
 	mdev->sriov.alias_guid.ports_guid[port->num - 1].all_rec_per_port[record_num].guid_indexes
 		|= mlx4_ib_get_aguid_comp_mask_from_ix(guid_index_in_rec);
-- 
1.7.1

--
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

[PATCH for-next 6/9] IB/mlx4: Request alias GUID on demand

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Request GIDs from the SM on demand, i.e., when a VF actually needs them,
and release them when the GIDs are no longer in use.

In cloud environments, this is useful for GID migrations, in which a
GID is assigned to a VF on the destination HCA, while the VF on the
source HCA is shutdown (but the GID was not administratively released).

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/alias_GUID.c |   51 +++++++++++++++++++++++++++++++
 drivers/infiniband/hw/mlx4/main.c       |   22 +++++++++++++
 drivers/infiniband/hw/mlx4/mlx4_ib.h    |    2 +
 3 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/alias_GUID.c b/drivers/infiniband/hw/mlx4/alias_GUID.c
index cbadab0..f5935c5 100644
--- a/drivers/infiniband/hw/mlx4/alias_GUID.c
+++ b/drivers/infiniband/hw/mlx4/alias_GUID.c
@@ -123,6 +123,57 @@ ib_sa_comp_mask mlx4_ib_get_aguid_comp_mask_from_ix(int index)
 	return IB_SA_COMP_MASK(4 + index);
 }
 
+void mlx4_ib_slave_alias_guid_event(struct mlx4_ib_dev *dev, int slave,
+				    int port,  int slave_init)
+{
+	__be64 curr_guid, required_guid;
+	int record_num = slave / 8;
+	int index = slave % 8;
+	int port_index = port - 1;
+	unsigned long flags;
+	int do_work = 0;
+
+	spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags);
+	if (dev->sriov.alias_guid.ports_guid[port_index].state_flags &
+	    GUID_STATE_NEED_PORT_INIT)
+		goto unlock;
+	if (!slave_init) {
+		curr_guid = *(__be64 *)&dev->sriov.
+			alias_guid.ports_guid[port_index].
+			all_rec_per_port[record_num].
+			all_recs[GUID_REC_SIZE * index];
+		if (curr_guid == cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL) ||
+		    !curr_guid)
+			goto unlock;
+		required_guid = cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL);
+	} else {
+		required_guid = mlx4_get_admin_guid(dev->dev, slave, port);
+		if (required_guid == cpu_to_be64(MLX4_GUID_FOR_DELETE_VAL))
+			goto unlock;
+	}
+	*(__be64 *)&dev->sriov.alias_guid.ports_guid[port_index].
+		all_rec_per_port[record_num].
+		all_recs[GUID_REC_SIZE * index] = required_guid;
+	dev->sriov.alias_guid.ports_guid[port_index].
+		all_rec_per_port[record_num].guid_indexes
+		|= mlx4_ib_get_aguid_comp_mask_from_ix(index);
+	dev->sriov.alias_guid.ports_guid[port_index].
+		all_rec_per_port[record_num].status
+		= MLX4_GUID_INFO_STATUS_IDLE;
+	/* set to run immediately */
+	dev->sriov.alias_guid.ports_guid[port_index].
+		all_rec_per_port[record_num].time_to_run = 0;
+	dev->sriov.alias_guid.ports_guid[port_index].
+		all_rec_per_port[record_num].
+		guids_retry_schedule[index] = 0;
+	do_work = 1;
+unlock:
+	spin_unlock_irqrestore(&dev->sriov.alias_guid.ag_work_lock, flags);
+
+	if (do_work)
+		mlx4_ib_init_alias_guid_work(dev, port_index);
+}
+
 /*
  * Whenever new GUID is set/unset (guid table change) create event and
  * notify the relevant slave (master also should be notified).
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index b972c0b..35f00ae 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2790,9 +2790,31 @@ static void mlx4_ib_event(struct mlx4_dev *dev, void *ibdev_ptr,
 	case MLX4_DEV_EVENT_SLAVE_INIT:
 		/* here, p is the slave id */
 		do_slave_init(ibdev, p, 1);
+		if (mlx4_is_master(dev)) {
+			int i;
+
+			for (i = 1; i <= ibdev->num_ports; i++) {
+				if (rdma_port_get_link_layer(&ibdev->ib_dev, i)
+					== IB_LINK_LAYER_INFINIBAND)
+					mlx4_ib_slave_alias_guid_event(ibdev,
+								       p, i,
+								       1);
+			}
+		}
 		return;
 
 	case MLX4_DEV_EVENT_SLAVE_SHUTDOWN:
+		if (mlx4_is_master(dev)) {
+			int i;
+
+			for (i = 1; i <= ibdev->num_ports; i++) {
+				if (rdma_port_get_link_layer(&ibdev->ib_dev, i)
+					== IB_LINK_LAYER_INFINIBAND)
+					mlx4_ib_slave_alias_guid_event(ibdev,
+								       p, i,
+								       0);
+			}
+		}
 		/* here, p is the slave id */
 		do_slave_init(ibdev, p, 0);
 		return;
diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h b/drivers/infiniband/hw/mlx4/mlx4_ib.h
index 532a877..fce3934 100644
--- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
+++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
@@ -798,6 +798,8 @@ int add_sysfs_port_mcg_attr(struct mlx4_ib_dev *device, int port_num,
 void del_sysfs_port_mcg_attr(struct mlx4_ib_dev *device, int port_num,
 			     struct attribute *attr);
 ib_sa_comp_mask mlx4_ib_get_aguid_comp_mask_from_ix(int index);
+void mlx4_ib_slave_alias_guid_event(struct mlx4_ib_dev *dev, int slave,
+				    int port, int slave_init);
 
 int mlx4_ib_device_register_sysfs(struct mlx4_ib_dev *device) ;
 
-- 
1.7.1

--
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

[PATCH for-next 7/9] net/mlx4_core: Raise slave shutdown event upon FLR

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

There might be cases that PF doesn't get a "reset" command upon slave down
(e.g. virsh destroy). In these cases, however, an FLR event is issued.

Therefore, when the PF receives an FLR event for a slave, it should also
generate a shutdown event on the PF for that slave, to let the PF upper
layers (mlx4_ib, eth) perform any required cleanup/actions associated
with slave shutdown.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx4/eq.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index 264bc15..901a0c6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -706,6 +706,8 @@ static int mlx4_eq_int(struct mlx4_dev *dev, struct mlx4_eq *eq)
 				priv->mfunc.master.slave_state[flr_slave].is_slave_going_down = 1;
 			}
 			spin_unlock_irqrestore(&priv->mfunc.master.slave_state_lock, flags);
+			mlx4_dispatch_event(dev, MLX4_DEV_EVENT_SLAVE_SHUTDOWN,
+					    flr_slave);
 			queue_work(priv->mfunc.master.comm_wq,
 				   &priv->mfunc.master.slave_flr_event_work);
 			break;
-- 
1.7.1

--
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

[PATCH for-next 8/9] net/mlx4_core: Return the admin alias GUID upon host view request

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Return the admin alias GUID value upon a GET request via HOST. We do this so
that the GUID value requested by the admin is returned even if the SM has not
yet approved this GUID (e.g. the SM is down).

Note that this does not create a problem, since the virtual port will remain
down until the SM does ACK the requested GUID value.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c |   41 +++++++++++++++++++----------
 1 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 778de74..bcc6b2d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -936,21 +936,34 @@ static int mlx4_MAD_IFC_wrapper(struct mlx4_dev *dev, int slave,
 				return err;
 			}
 			if (smp->attr_id == IB_SMP_ATTR_GUID_INFO) {
-				/* compute slave's gid block */
-				smp->attr_mod = cpu_to_be32(slave / 8);
-				/* execute cmd */
-				err = mlx4_cmd_box(dev, inbox->dma, outbox->dma,
-					     vhcr->in_modifier, opcode_modifier,
-					     vhcr->op, MLX4_CMD_TIME_CLASS_C, MLX4_CMD_NATIVE);
-				if (!err) {
-					/* if needed, move slave gid to index 0 */
-					if (slave % 8)
-						memcpy(outsmp->data,
-						       outsmp->data + (slave % 8) * 8, 8);
-					/* delete all other gids */
-					memset(outsmp->data + 8, 0, 56);
+				__be64 guid = mlx4_get_admin_guid(dev, slave,
+								  port);
+
+				/* set the PF admin guid to the FW/HW burned
+				 * GUID, if it wasn't yet set
+				 */
+				if (slave == 0 && guid == 0) {
+					smp->attr_mod = 0;
+					err = mlx4_cmd_box(dev,
+							   inbox->dma,
+							   outbox->dma,
+							   vhcr->in_modifier,
+							   opcode_modifier,
+							   vhcr->op,
+							   MLX4_CMD_TIME_CLASS_C,
+							   MLX4_CMD_NATIVE);
+					if (err)
+						return err;
+					mlx4_set_admin_guid(dev,
+							    *(__be64 *)outsmp->
+							    data, slave, port);
+				} else {
+					memcpy(outsmp->data, &guid, 8);
 				}
-				return err;
+
+				/* clean all other gids */
+				memset(outsmp->data + 8, 0, 56);
+				return 0;
 			}
 			if (smp->attr_id == IB_SMP_ATTR_NODE_INFO) {
 				err = mlx4_cmd_box(dev, inbox->dma, outbox->dma,
-- 
1.7.1

--
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

[PATCH for-next 9/9] IB/mlx4: Change alias guids default to be host assigned

[Or Gerlitz]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Change the default mode to be HOST assigned instead of SM assigned. This is
the expected operational mode, because it doesn't depend on SM availability.

As PF generates random GUIDs as the initial admin values, this gives
out of the box experience.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 35f00ae..7a55fee 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -66,9 +66,9 @@ MODULE_DESCRIPTION("Mellanox ConnectX HCA InfiniBand driver");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(DRV_VERSION);
 
-int mlx4_ib_sm_guid_assign = 1;
+int mlx4_ib_sm_guid_assign = 0;
 module_param_named(sm_guid_assign, mlx4_ib_sm_guid_assign, int, 0444);
-MODULE_PARM_DESC(sm_guid_assign, "Enable SM alias_GUID assignment if sm_guid_assign > 0 (Default: 1)");
+MODULE_PARM_DESC(sm_guid_assign, "Enable SM alias_GUID assignment if sm_guid_assign > 0 (Default: 0)");
 
 static const char mlx4_ib_version[] =
 	DRV_NAME ": Mellanox ConnectX InfiniBand driver v"
-- 
1.7.1

--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Or Gerlitz]

On Sun, Mar 29, 2015 at 4:51 PM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> Under the existing implementation for virtual GIDs, if the SM is not
> reachable or incurs a delayed response, or if the VF is probed into a
> VM before their GUID is registered with the SM, there exists a window
> in time in which the VF sees an incorrect GID, i.e., not the GID that
> was intended by the admin. This results in exposing a temporal identity
> to the VF.

Hi Roland, so your for-next branch is again way behind, still on 3.19
and while 4.0 is soon @ rc6, we couldn't even rebase this series on
it. It's really hard where your tree is really active once every nine
weeks or so, e.g only few days before/after rc1's. I'm not sure what
you expect us to do, kernel development simply needs not be like this.

April 3rd-12th is holiday here, and we would like to really really
know early this week what you intend to pull for 4.1 out of the
pending things in linux-rdma.

Or.


> Moreover, a subsequent change in the alias GID causes a spec-incompliant
> change to the VF identity. Some guest operating systems, such as Windows,
> cannot tolerate such changes.
>
> This series solves above problem by exposing the admin desired value instead
> of the value that was approved by the SM. As long as the SM doesn't approve
> the GID, the VF would see its link as down.
>
> In addition, we request GIDs from the SM on demand, i.e., when a VF actually
> needs them, and release them when the GIDs are no longer in use. In cloud
> environments, this is useful for GID migrations, in which a GID is assigned to
> a VF on the destination HCA, while the VF on the source HCA is shut down (but
> the GID was not administratively released).
>
> For reasons of compatibility, an explicit admin request to set/change a GUID
> entry is done immediately, regardless of whether the VF is active or not. This
> allows administrators to change the GUID without the need to unbind/bind the VF.
>
> In addition, the existing implementation doesn't support a persistency
> mechanism to retry a GUID request when the SM has rejected it for any reason.
> The PF driver shall keep trying to acquire the specified GUID indefinitely by
> utilizing an exponential back off scheme, this should be managed per GUID and
> be aligned with other incoming admin requests.
>
> This ability needed especially for the on-demand GUID feature. In this case, we
> must manage the GUID's status per entry and handle cases that some entries are
> temporarily rejected.
>
> The first patch adds the persistency support and is pre-requisites for the
> series.  Further patches make the change to use the admin VF behavior as
> described above.
>
> Finally, the default mode is changed to be HOST assigned instead of SM
> assigned. This is the expected operational mode, because it doesn't depend on
> SM availability as described above.
>
> Yishai and Or.
>
> Yishai Hadas (9):
>   IB/mlx4: Alias GUID adding persistency support
>   net/mlx4_core: Manage alias GUID per VF
>   net/mlx4_core: Set initial admin GUIDs for VFs
>   IB/mlx4: Manage admin alias GUID upon admin request
>   IB/mlx4: Change init flow to request alias GUIDs for active VFs
>   IB/mlx4: Request alias GUID on demand
>   net/mlx4_core: Raise slave shutdown event upon FLR
>   net/mlx4_core: Return the admin alias GUID upon host view request
>   IB/mlx4: Change alias guids default to be host assigned
>
>  drivers/infiniband/hw/mlx4/alias_GUID.c   |  468 +++++++++++++++++++++--------
>  drivers/infiniband/hw/mlx4/main.c         |   26 ++-
>  drivers/infiniband/hw/mlx4/mlx4_ib.h      |   14 +-
>  drivers/infiniband/hw/mlx4/sysfs.c        |   44 +--
>  drivers/net/ethernet/mellanox/mlx4/cmd.c  |   42 ++-
>  drivers/net/ethernet/mellanox/mlx4/eq.c   |    2 +
>  drivers/net/ethernet/mellanox/mlx4/main.c |   39 +++
>  drivers/net/ethernet/mellanox/mlx4/mlx4.h |    1 +
>  include/linux/mlx4/device.h               |    4 +
>  9 files changed, 459 insertions(+), 181 deletions(-)
>
> --
> 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
--
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

Re: [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs

[Jason Gunthorpe]

On Sun, Mar 29, 2015 at 04:51:27PM +0300, Or Gerlitz wrote:
> +void mlx4_set_random_admin_guid(struct mlx4_dev *dev, int entry, int port)
> +{
> +	struct mlx4_priv *priv = mlx4_priv(dev);
> +	u8 random_mac[6];
> +	char *raw_gid;
> +
> +	/* hw GUID */
> +	if (entry == 0)
> +		return;
> +
> +	eth_random_addr(random_mac);
> +	raw_gid = (char *)&priv->mfunc.master.vf_admin[entry].vport[port].guid;

raw_gid is actually a guid

> +	raw_gid[0] = random_mac[0] ^ 2;

eth_random_addr already guarentees the ULA bit is set to one (local),
so this is wrong. IBA uses the EUI-64 system, not the IPv6
modification.

> +	raw_gid[1] = random_mac[1];
> +	raw_gid[2] = random_mac[2];
> +	raw_gid[3] = 0xff;
> +	raw_gid[4] = 0xfe;

This should be 0xff for mapping a MAC to a EUI-64

But, it doesn't really make sense to use eth_random_addr (which
doesn't have a special OUI) and not randomize every bit.

        get_random_bytes(&guid, sizeof(guid));
        guid &= ~(1ULL << 56);
	guid |= 1ULL << 57;

I also don't think the kernel should be generating random GUIDs. Either
the SA should be consulted to do this, or the management stack should
generate a cloud wide unique number.

Jason
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[David Miller]

From: Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Mon, 30 Mar 2015 19:17:01 +0300

> On Sun, Mar 29, 2015 at 4:51 PM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> Under the existing implementation for virtual GIDs, if the SM is not
>> reachable or incurs a delayed response, or if the VF is probed into a
>> VM before their GUID is registered with the SM, there exists a window
>> in time in which the VF sees an incorrect GID, i.e., not the GID that
>> was intended by the admin. This results in exposing a temporal identity
>> to the VF.
> 
> Hi Roland, so your for-next branch is again way behind, still on 3.19
> and while 4.0 is soon @ rc6, we couldn't even rebase this series on
> it. It's really hard where your tree is really active once every nine
> weeks or so, e.g only few days before/after rc1's. I'm not sure what
> you expect us to do, kernel development simply needs not be like this.
> 
> April 3rd-12th is holiday here, and we would like to really really
> know early this week what you intend to pull for 4.1 out of the
> pending things in linux-rdma.

Roland, I have to genuinely agree with Or, that your handling of
patch integration is sub-par and really painful for anyone actually
trying to get real work done here.

If you simply don't have the time to devote to constantly reviewing
patches as they come in, and doing so in a timely manner, please let
someone who is actually interested and has the time to take over.

Only integrating peoples work right before the merge window, and then
disappearing for a long time really isn't acceptable.

Thanks!
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Roland Dreier]

> Roland, I have to genuinely agree with Or, that your handling of
> patch integration is sub-par and really painful for anyone actually
> trying to get real work done here.
>
> If you simply don't have the time to devote to constantly reviewing
> patches as they come in, and doing so in a timely manner, please let
> someone who is actually interested and has the time to take over.

It's a fair criticism, and certainly for at least the last year or so
I have not had the time to do enough work as a maintainer.  I have
hope that some of the things that have been keeping me busy are dying
down and that I'll have more time to spend on handling the RDMA tree,
but that's just talk until I actually get more done.

I really would like to get more people involved in handling the flow
of patches but I'm not sure who has not only the interest and the time
but also the judgement and expertise to take over.  Certainly Or has
been a long time contributor who has done a lot of great things, but I
still worry about things like ABI stability and backwards
compatibility.

But I'm open to ideas.

 - R.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Sagi Grimberg]

On 3/31/2015 6:47 AM, Roland Dreier wrote:
>> Roland, I have to genuinely agree with Or, that your handling of
>> patch integration is sub-par and really painful for anyone actually
>> trying to get real work done here.
>>
>> If you simply don't have the time to devote to constantly reviewing
>> patches as they come in, and doing so in a timely manner, please let
>> someone who is actually interested and has the time to take over.
>
> It's a fair criticism, and certainly for at least the last year or so
> I have not had the time to do enough work as a maintainer.  I have
> hope that some of the things that have been keeping me busy are dying
> down and that I'll have more time to spend on handling the RDMA tree,
> but that's just talk until I actually get more done.
>
> I really would like to get more people involved in handling the flow
> of patches but I'm not sure who has not only the interest and the time
> but also the judgement and expertise to take over.  Certainly Or has
> been a long time contributor who has done a lot of great things, but I
> still worry about things like ABI stability and backwards
> compatibility.
>
> But I'm open to ideas.
>

We had a talk about a similar topic at LSFMM15. Linux-scsi subsystem is 
a large scale subsystem and also has the "single maintainer with
limited time for upstream maintenance" bottleneck.

Christoph created the "scsi-queue" tree to feed James in order to
accelerate the work process.

One thought was laying a set of rules that would allow a maintainer
to "just apply patches":
- Obviously applies cleanly and does not produce compilation
errors/warning (patches that don't meet this will be removed)
- At least two Reviewed-by tags (one of them can be Acked-by/Tested-by)

One problem with that is that we need "a christoph" to poke people for
review since not a lot of people giving review.

Another thought was to allow multiple maintainers (my understanding is
the patchwork supports it). If we can find some split to allow different
maintainers to feed Roland this might work.

Thoughts?

Sagi.
--
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

Re: [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs

[Or Gerlitz]

On 3/30/2015 8:16 PM, Jason Gunthorpe wrote:
> But, it doesn't really make sense to use eth_random_addr (which
> doesn't have a special OUI) and not randomize every bit.
>
>          get_random_bytes(&guid, sizeof(guid));
>          guid &= ~(1ULL << 56);
> 	guid |= 1ULL << 57;

OK, we can do that, sure.

> I also don't think the kernel should be generating random GUIDs. Either the SA should be consulted to do this, or the management stack should generate a cloud wide unique number.

There **is** an option to consult with the SM controlled by the 
sm_guid_assign module param of mlx4_ib. When this mode is in action, the 
randomized guid would be overridden by the one assigned by the SA.

In real life environments, people will use the "host assigned" option 
(sm_guid_assign = false) where the virtualization systems generate a 
unique guid to be used by the VF/VM and provision it through mlx4_ib to 
be provided to VF when the bindin.

The only place where the random value will be used, is for OOTB 
experience for staging purposes and such, when one wants to have working 
system with zero-touch. The practice of generating locally random 
address for non-provisioned VF is also used by the pretty much the whole 
set of Ethernet VF drivers (ixgbe_vf, i40e_vf, mlx4 VF code, etc). This 
is low cost solution for point cases which need that.

Or.

--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Or Gerlitz]

On Tue, Mar 31, 2015 at 6:47 AM, Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> Roland, I have to genuinely agree with Or, that your handling of
>> patch integration is sub-par and really painful for anyone actually
>> trying to get real work done here.
>>
>> If you simply don't have the time to devote to constantly reviewing
>> patches as they come in, and doing so in a timely manner, please let
>> someone who is actually interested and has the time to take over.
>
> It's a fair criticism, and certainly for at least the last year or so
> I have not had the time to do enough work as a maintainer.  I have
> hope that some of the things that have been keeping me busy are dying
> down and that I'll have more time to spend on handling the RDMA tree,
> but that's just talk until I actually get more done.

Roland, well, with few variations, this goes way beyond a year. I
would say more or less half of the time you're wearing the maintainer
hat (2005-now). The practice of updating your for-next branch to rc1
only few days/hours before the the kernel is out and the merge window
opens, is an attitude, not lack of resources and will not be solved by
whatever processes people suggest. You need to act differently.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Christoph Lameter]

Well this needs to be addressed yes but in order to have that done someone
needs to step forward do the proper work, maintain git trees, do the
review and show up for the conferences. Neither Roland nor Or was there at
the Infiniband conferences in Monterey this year(!) and this is the prime
venue for face to face conversation about the subsystem each year.

Just looked at the MAINTAINERS file: Surprise: We already have 3
maintainers for the IB subsystem:

INFINIBAND SUBSYSTEM
M:      Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
M:      Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
M:      Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
L:      linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
W:      http://www.openfabrics.org/
Q:      http://patchwork.kernel.org/project/linux-rdma/list/
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git
S:      Supported
F:      Documentation/infiniband/
F:      drivers/infiniband/
F:      include/uapi/linux/if_infiniband.h

Sean and Hal showed up for the conference. Sean is active mostly in
focusing on userspace stuff. Both could become more involved in the kernel
ib tree and the review process? Hal is employed by Mellanox but is not
participating in these patch related discussions? Can Mellanox give Hal
more time for his role as the kernel ib subsystem maintainer? Maybe you
could get that resolved at Mellanox? You already have one of your own as
a maintainer of the IB subsystem.


--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Hal Rosenstock]

On 3/31/2015 8:49 AM, Christoph Lameter wrote:
> Well this needs to be addressed yes but in order to have that done someone
> needs to step forward do the proper work, maintain git trees, do the
> review and show up for the conferences. Neither Roland nor Or was there at
> the Infiniband conferences in Monterey this year(!) and this is the prime
> venue for face to face conversation about the subsystem each year.
> 
> Just looked at the MAINTAINERS file: Surprise: We already have 3
> maintainers for the IB subsystem:
> 
> INFINIBAND SUBSYSTEM
> M:      Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> M:      Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> M:      Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> L:      linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> W:      http://www.openfabrics.org/
> Q:      http://patchwork.kernel.org/project/linux-rdma/list/
> T:      git git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git
> S:      Supported
> F:      Documentation/infiniband/
> F:      drivers/infiniband/
> F:      include/uapi/linux/if_infiniband.h
> 
> Sean and Hal showed up for the conference. Sean is active mostly in
> focusing on userspace stuff. Both could become more involved in the kernel
> ib tree and the review process? Hal is employed by Mellanox but is not
> participating in these patch related discussions? Can Mellanox give Hal
> more time for his role as the kernel ib subsystem maintainer? Maybe you
> could get that resolved at Mellanox? You already have one of your own as
> a maintainer of the IB subsystem.

My involvement is limited to IB management related aspects in the kernel.

-- Hal

--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[David Miller]

From: Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Tue, 31 Mar 2015 14:22:50 +0300

> On Tue, Mar 31, 2015 at 6:47 AM, Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> Roland, I have to genuinely agree with Or, that your handling of
>>> patch integration is sub-par and really painful for anyone actually
>>> trying to get real work done here.
>>>
>>> If you simply don't have the time to devote to constantly reviewing
>>> patches as they come in, and doing so in a timely manner, please let
>>> someone who is actually interested and has the time to take over.
>>
>> It's a fair criticism, and certainly for at least the last year or so
>> I have not had the time to do enough work as a maintainer.  I have
>> hope that some of the things that have been keeping me busy are dying
>> down and that I'll have more time to spend on handling the RDMA tree,
>> but that's just talk until I actually get more done.
> 
> Roland, well, with few variations, this goes way beyond a year. I
> would say more or less half of the time you're wearing the maintainer
> hat (2005-now). The practice of updating your for-next branch to rc1
> only few days/hours before the the kernel is out and the merge window
> opens, is an attitude, not lack of resources and will not be solved by
> whatever processes people suggest. You need to act differently.

Unfortunately, and no direct offense intend to you personally Roland,
but I agree with Or here.

If a person really cares about a subsystem they are marked at the
maintainer for, they usually _make_ the time necessary to apply
patches and attend to maintainership in a reasonable manner.

If this "once every merge window" behavior was limited to one release
cycle, I'd give the benefit of the doubt, but this has been going on
for a very long time.

You cannot on the one hand say "I care about this subsystem and the
long term maintainership and ABI stability of it" yet on the other
hand not be willing to put forth even the _MOST MINIMAL_ amount of
effort and time required to steadily review and apply patches over a
period of several years.

It doesn't take a lot of time to do this work, especially if you use
the correct tools.  I can review and apply 100 patches a day, even
when I'm on vacation.  I'm an extreme example, but what you're doing
right now Roland is not acceptable and is not in agreement with your
claims about how much you care about this subsystem.

If you processed the incoming patch queue even _once_ a week, we
wouldn't even be having this conversation.  But you haven't even been
doing that.

I get sick to my stomache when a patch gets to 3 days in my patch
queue, that's already too long.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Christoph Lameter]

On Tue, 31 Mar 2015, Hal Rosenstock wrote:

> > Sean and Hal showed up for the conference. Sean is active mostly in
> > focusing on userspace stuff. Both could become more involved in the kernel
> > ib tree and the review process? Hal is employed by Mellanox but is not
> > participating in these patch related discussions? Can Mellanox give Hal
> > more time for his role as the kernel ib subsystem maintainer? Maybe you
> > could get that resolved at Mellanox? You already have one of your own as
> > a maintainer of the IB subsystem.
>
> My involvement is limited to IB management related aspects in the kernel.

Need to update the entry then I think.

--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[David Miller]

From: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
Date: Tue, 31 Mar 2015 07:49:38 -0500 (CDT)

> Well this needs to be addressed yes but in order to have that done someone
> needs to step forward do the proper work, maintain git trees, do the
> review and show up for the conferences. Neither Roland nor Or was there at
> the Infiniband conferences in Monterey this year(!) and this is the prime
> venue for face to face conversation about the subsystem each year.

I don't go to IETF meetings, ever, yet I am rather sure that nobody
uses that to question my ability to be the networking maintainer.

I've gone several years at times without meeting other networking
developers as well, and that also I am pretty sure did not harm my
stature as the networking maintainer.

So I absolutely disagree that these two acts are requirements for
someone whose job is to steadily and reasonably review and apply
patches for a subsystem.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Christoph Lameter]

On Tue, 31 Mar 2015, David Miller wrote:

> > Well this needs to be addressed yes but in order to have that done someone
> > needs to step forward do the proper work, maintain git trees, do the
> > review and show up for the conferences. Neither Roland nor Or was there at
> > the Infiniband conferences in Monterey this year(!) and this is the prime
> > venue for face to face conversation about the subsystem each year.
>
> I don't go to IETF meetings, ever, yet I am rather sure that nobody
> uses that to question my ability to be the networking maintainer.

This isnt a standards body. Openfabrics is the organization dedicated to
the development of the infiniband stack.

--
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

RE: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Hefty, Sean]

> It doesn't take a lot of time to do this work, especially if you use
> the correct tools.  I can review and apply 100 patches a day, even
> when I'm on vacation.  I'm an extreme example, but what you're doing
> right now Roland is not acceptable and is not in agreement with your
> claims about how much you care about this subsystem.

I honestly think part of the issue here is general approach being taken by the rdma stack, which is to expose every hardware specific component that a vendor might define through the interfaces up to the consumers.  This results in unwieldy interfaces that are under constant churn, with no clear direction.  If most submissions to the rdma stack were contained the lower-level drivers, I don't know that we would have the issues that we do.  But a significant percentage of rdma patches modify the rdma core software layer, which threaten the stability of the entire stack.

- Sean
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Jason Gunthorpe]

On Tue, Mar 31, 2015 at 12:13:58PM +0300, Sagi Grimberg wrote:

> We had a talk about a similar topic at LSFMM15. Linux-scsi subsystem
> is a large scale subsystem and also has the "single maintainer with
> limited time for upstream maintenance" bottleneck.
> 
> Christoph created the "scsi-queue" tree to feed James in order to
> accelerate the work process.

[..]

> One problem with that is that we need "a christoph" to poke people for
> review since not a lot of people giving review.

I have tried for a few years to get enough interested/qualified people
together to do this kind of system, but it just hasn't happened.

It feels like we are stuck in a loop: People and companies *do not*
want to work on drivers/infiniband because too often patches just die.
Asking people to work on it, and companies to fund work, gets met with
the above.

I think there is a broad agreement we need to change the system
here. So, lets hear from people and companies: put your name forward,
consistently provide time to do serious review work on all patches.

Jason
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Or Gerlitz]

On Tue, Mar 31, 2015 at 8:27 PM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:


> I honestly think part of the issue here is general approach being taken by the rdma stack, which is to expose every hardware specific component that a vendor might define through the interfaces up to the consumers.

Sean,

I must disagree. You made this claim back when we submitted the IB
core patch which adds support for signature, protection etc offloads
(commit 1b01d33560e7 "IB/core: Introduce signature verbs API" and the
detailed replied you got were explaining that

1. this allows to offload SCSI T10 signature for RDMA block storage
protocols such as SRP and iSER
2. SCSI, SRP and iSER are all industry standards
3. T10 offload is done also by competing technologies such as Fibre-Channel
4. nothing, but exactly nothing in the verbs API is tied to specific
HCA implementation
5. etc

Very similar arguments would apply to the ODP submission.

So examples please!

Or.

This results in unwieldy interfaces that are under constant churn,
with no clear direction.  If most submissions to the rdma stack were
contained the lower-level drivers, I don't know that we would have the
issues that we do.  But a significant percentage of rdma patches
modify the rdma core software layer, which threaten the stability of
the entire stack.
--
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

RE: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Hefty, Sean]

> I must disagree. You made this claim back when we submitted the IB
> core patch which adds support for signature, protection etc offloads
> (commit 1b01d33560e7 "IB/core: Introduce signature verbs API" and the
> detailed replied you got were explaining that

And I'll continue to make this claim because it is true.

> Very similar arguments would apply to the ODP submission.

Yes - ODP will _have_ to change the core.  USNIC had to change the core, and had changes rejected that made perfect sense conceptually.  This is part of the problem!!!   If you disagree, then submit patches that don't touch the core.

> So examples please!

Sure - this is from Somnath's latest patch series:

Matan Barak (14):
  IB/core: Add RoCE GID cache
  IB/core: Add kref to IB devices
  IB/core: Add RoCE GID population
  IB/core: Add default GID for RoCE GID Cache
  net/bonding: make DRV macros private
  net: Add info for NETDEV_CHANGEUPPER event
  IB/core: Add RoCE cache bonding support
  IB/core: GID attribute should be returned from verbs API and cache API
  IB/core: Report gid_type and gid_ndev through sysfs
  IB/core: Support find sgid index using a filter function
  IB/core: Modify ib_verbs and cma in order to use roce_gid_cache
  IB/core: Add gid_type to path and rdma_id_private
  IB/core: Add rdma_network_type to wc
  IB/cma: Add configfs for rdma_cm

Moni Shoua (13):
  IB/mlx4: Remove gid table management for RoCE
  IB/mlx4: Replace spin_lock with rw_semaphore
  IB/mlx4: Lock with RCU instead of RTNL
  net/mlx4: Postpone the registration of net_device
  IB/mlx4: Advertise RoCE support in port capabilities
  IB/mlx4: Implement ib_device callback - get_netdev
  IB/mlx4: Implement ib_device callback - modify_gid
  IB/mlx4: Configure device to work in RoCEv2
  IB/mlx4: Translate cache gid index to real index
  IB/core: Initialize UD header structure with IP and UDP headers
  IB/mlx4: Enable send of RoCE QP1 packets with IP/UDP headers
  IB/mlx4: Create and use another QP1 for RoCEv2
  IB/cma: Join and leave multicast groups with IGMP

That's a significant number of patches that modify the core rdma layer.  NFSoRDMA had 5 different ways to register memory.

I agree that Roland's response time is ridiculously slow.  But he does tend to merge in new drivers and updates that only touch a single vendor's driver fairly quickly.  It's the thrashing on the core that sees significant delays.

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Or Gerlitz]

On Wed, Apr 1, 2015 at 12:33 AM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> I must disagree. You made this claim back when we submitted the IB
>> core patch which adds support for signature, protection etc offloads
>> (commit 1b01d33560e7 "IB/core: Introduce signature verbs API" and the
>> detailed replied you got were explaining that
>
> And I'll continue to make this claim because it is true.
>
>> Very similar arguments would apply to the ODP submission.
>
> Yes - ODP will _have_ to change the core.  USNIC had to change the core, and had changes rejected that made perfect sense conceptually.  This is part of the problem!!!   If you disagree, then submit patches that don't touch the core.

Sean,

ODP (On Demand Paging)

8ada2c1 IB/core: Add support for on demand paging regions
860f10a IB/core: Add flags for on demand paging support

is upstream for few releases already, and it changed the core indeed.
Memory Registration is sort of PITA for RDMA consumers and ODP is a
brilliant concept which down the road should be removing the hassle of
memory registration altogether (think on transparent lkey per process,
but days will tell).

As you pointed out during the USNIC review, it's sort of hack which is
not supported by any standard, unlike the other supported RDMA
technologies which are upstream: IB, RoCE and iWARP. But guess what,
the RDMA maintainer made no comments, and silently picked it up.

Your claim says more or less "don't touch the good old
include/rdma/ib_verbs.h from 2.6.12, just add new low-level drivers"
-- this is very close to claiming that it doesn't make sense to change
anything in the low areas of the core networking stack or in
netdevice.h over the years, just add new Ethernet drivers. This does
not make any sense.

There are more and more new use cases for RDMA and indeed a nice
challenge to frame them generally with modified/new verbs APIs and
changes to the IB core, such as the patches to support name-spaces to
make RDMA usable in containers which you (maintainer of the CM and
RDMA-CM in the IB core) is ignoring for couple of months too
(following Roland?).


>> So examples please!
> Sure - this is from Somnath's latest patch series:
>
> Matan Barak (14):
>   IB/core: Add RoCE GID cache
>   IB/core: Add kref to IB devices
>   IB/core: Add RoCE GID population
>   IB/core: Add default GID for RoCE GID Cache
>   net/bonding: make DRV macros private
>   net: Add info for NETDEV_CHANGEUPPER event
>   IB/core: Add RoCE cache bonding support
>   IB/core: GID attribute should be returned from verbs API and cache API
>   IB/core: Report gid_type and gid_ndev through sysfs
>   IB/core: Support find sgid index using a filter function
>   IB/core: Modify ib_verbs and cma in order to use roce_gid_cache
>   IB/core: Add gid_type to path and rdma_id_private
>   IB/core: Add rdma_network_type to wc
>   IB/cma: Add configfs for rdma_cm
>
> Moni Shoua (13):
>   IB/mlx4: Remove gid table management for RoCE
>   IB/mlx4: Replace spin_lock with rw_semaphore
>   IB/mlx4: Lock with RCU instead of RTNL
>   net/mlx4: Postpone the registration of net_device
>   IB/mlx4: Advertise RoCE support in port capabilities
>   IB/mlx4: Implement ib_device callback - get_netdev
>   IB/mlx4: Implement ib_device callback - modify_gid
>   IB/mlx4: Configure device to work in RoCEv2
>   IB/mlx4: Translate cache gid index to real index
>   IB/core: Initialize UD header structure with IP and UDP headers
>   IB/mlx4: Enable send of RoCE QP1 packets with IP/UDP headers
>   IB/mlx4: Create and use another QP1 for RoCEv2
>   IB/cma: Join and leave multicast groups with IGMP
>
> That's a significant number of patches that modify the core rdma layer.

This series indeed is a bit heavy as it brings three changes

1. move the RoCE GID table management from LL drivers (ocrdma and
mlx4) into the IB core
2. support multiple GID types
3. support RoCE V2

#1 is terribly making sense, b/c RoCE GID addresses are derived
through net events
from IP addresses configured to the  buddy Ethernet net-device and uppers (vlan,
bond and such) so there's no point to have this logic replicated over
and over in LL drivers.

#2 and #3 follow the IBTA spec of RoCE V2

It could be perfect maintainer comment to say: do it one-by-one, but
anybody there?

If you have concrete feedback on step #1 or anything else in the
series, let them know.

> NFSoRDMA had 5 different ways to register memory.

that's bad protocol implementation, so they are fixing it now. Has
nothing to do with the IB core.

> I agree that Roland's response time is ridiculously slow.  But he does tend to merge in new drivers and updates that only touch a single vendor's driver fairly quickly.  It's the thrashing on the core that sees significant delays.

Without ability to add the changed to the core, we can't make progress

Or.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[ira.weiny]

On Thu, Apr 02, 2015 at 05:32:53PM +0300, Or Gerlitz wrote:
> On Wed, Apr 1, 2015 at 12:33 AM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

[snip]

> 
> Your claim says more or less "don't touch the good old
> include/rdma/ib_verbs.h from 2.6.12, just add new low-level drivers"
> -- this is very close to claiming that it doesn't make sense to change
> anything in the low areas of the core networking stack or in
> netdevice.h over the years, just add new Ethernet drivers. This does
> not make any sense.

I don't think the question is "if" we should change the core but "how".

Seans point is that the core seems to be in constant flux.  Furthermore, Roland
and others have found enough problems with the core changes in the past that
they are _not_ comfortable applying them without serious review.  Many of the
changes proposed here are completely new and require serious time to
understand.

Most people on this list have limited time and are unable to review every
vendors hardware implementation.  What they do care about is how those changes
affect the core and how those core changes then affect their hardware, other
hardware they use, and the ULPs.  This becomes a huge amount of time.

To facilitate this we should be looking for ways to minimize and be very clear
the ramifications of the core changes.  In addition, we need to identify where
the core needs to be cleaned up such that future core changes are either 1)
unnecessary or 2) easily reviewable because of their limited impact to other
areas.


With all that said, I too must voice my concerns with Rolands lack of activity.


There have been some good discussions recently on re-architecting the device
feature indicators which were spawned from my OPA MAD changes.

Various alternatives have been submitted and discussed but Roland has not
weighed in on which are acceptable.  This makes it difficult to determine what
direction we should take.

Also, recently I found out my repo for the 0-day build was no longer testing my
branches because Rolands for-next branch was too old.  I see today that Roland
has updated to 4.0 rc now.  Thank you.


Ira


> There are more and more new use cases for RDMA and indeed a nice
> challenge to frame them generally with modified/new verbs APIs and
> changes to the IB core, such as the patches to support name-spaces to
> make RDMA usable in containers which you (maintainer of the CM and
> RDMA-CM in the IB core) is ignoring for couple of months too
> (following Roland?).
> 
> 
> >> So examples please!
> > Sure - this is from Somnath's latest patch series:
> >
> > Matan Barak (14):
> >   IB/core: Add RoCE GID cache
> >   IB/core: Add kref to IB devices
> >   IB/core: Add RoCE GID population
> >   IB/core: Add default GID for RoCE GID Cache
> >   net/bonding: make DRV macros private
> >   net: Add info for NETDEV_CHANGEUPPER event
> >   IB/core: Add RoCE cache bonding support
> >   IB/core: GID attribute should be returned from verbs API and cache API
> >   IB/core: Report gid_type and gid_ndev through sysfs
> >   IB/core: Support find sgid index using a filter function
> >   IB/core: Modify ib_verbs and cma in order to use roce_gid_cache
> >   IB/core: Add gid_type to path and rdma_id_private
> >   IB/core: Add rdma_network_type to wc
> >   IB/cma: Add configfs for rdma_cm
> >
> > Moni Shoua (13):
> >   IB/mlx4: Remove gid table management for RoCE
> >   IB/mlx4: Replace spin_lock with rw_semaphore
> >   IB/mlx4: Lock with RCU instead of RTNL
> >   net/mlx4: Postpone the registration of net_device
> >   IB/mlx4: Advertise RoCE support in port capabilities
> >   IB/mlx4: Implement ib_device callback - get_netdev
> >   IB/mlx4: Implement ib_device callback - modify_gid
> >   IB/mlx4: Configure device to work in RoCEv2
> >   IB/mlx4: Translate cache gid index to real index
> >   IB/core: Initialize UD header structure with IP and UDP headers
> >   IB/mlx4: Enable send of RoCE QP1 packets with IP/UDP headers
> >   IB/mlx4: Create and use another QP1 for RoCEv2
> >   IB/cma: Join and leave multicast groups with IGMP
> >
> > That's a significant number of patches that modify the core rdma layer.
> 
> This series indeed is a bit heavy as it brings three changes
> 
> 1. move the RoCE GID table management from LL drivers (ocrdma and
> mlx4) into the IB core
> 2. support multiple GID types
> 3. support RoCE V2
> 
> #1 is terribly making sense, b/c RoCE GID addresses are derived
> through net events
> from IP addresses configured to the  buddy Ethernet net-device and uppers (vlan,
> bond and such) so there's no point to have this logic replicated over
> and over in LL drivers.
> 
> #2 and #3 follow the IBTA spec of RoCE V2
> 
> It could be perfect maintainer comment to say: do it one-by-one, but
> anybody there?
> 
> If you have concrete feedback on step #1 or anything else in the
> series, let them know.
> 
> > NFSoRDMA had 5 different ways to register memory.
> 
> that's bad protocol implementation, so they are fixing it now. Has
> nothing to do with the IB core.
> 
> > I agree that Roland's response time is ridiculously slow.  But he does tend to merge in new drivers and updates that only touch a single vendor's driver fairly quickly.  It's the thrashing on the core that sees significant delays.
> 
> Without ability to add the changed to the core, we can't make progress
> 
> Or.
> --
> 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
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Or Gerlitz]

On Fri, Apr 3, 2015 at 1:31 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Apr 02, 2015 at 05:32:53PM +0300, Or Gerlitz wrote:
>> On Wed, Apr 1, 2015 at 12:33 AM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
> [snip]

>> Your claim says more or less "don't touch the good old
>> include/rdma/ib_verbs.h from 2.6.12, just add new low-level drivers"
>> -- this is very close to claiming that it doesn't make sense to change
>> anything in the low areas of the core networking stack or in
>> netdevice.h over the years, just add new Ethernet drivers. This does
>> not make any sense.

> I don't think the question is "if" we should change the core but "how".

> Seans point is that the core seems to be in constant flux.  Furthermore, Roland
> and others have found enough problems with the core changes in the past that
> they are _not_ comfortable applying them without serious review.

Ira, examples please for core changes that went in and later turned to
be problematic?! I refer to APIs and structural changes that turned to
be such (not or to less extent point bugs, which should be avoided
too, you know..)

> Many of the changes proposed here are completely new and require serious
> time to understand.
> Most people on this list have limited time and are unable to review every
> vendors hardware implementation.  What they do care about is how those
> changes
> affect the core and how those core changes then affect their hardware, other
> hardware they use, and the ULPs.  This becomes a huge amount of time.

You're mixing between vendor's driver code to core changes -- "unable
to review every vendors hardware implementation" -- is clear and we
didn't ask for that.

As for what's involved in merging core API changes - you made good
description of what's needed there --- but this is all about the
development and evolution of a core stack for domain X (networking,
block storage, virtualization, RDMA or you namde it). Such an argument
must not be used, since can kill X's stack development and the rdma
case, leave us with the 10y old 2.6.12 based verbs header file.

> To facilitate this we should be looking for ways to minimize and be very clear
> the ramifications of the core changes.  In addition, we need to identify where
> the core needs to be cleaned up such that future core changes are either 1)
> unnecessary or 2) easily reviewable because of their limited impact to other
> areas.

I am not sure to follow on the core cleanup proposal, but open for
suggestions / ideas, please elaborate on this little further.

> With all that said, I too must voice my concerns with Rolands lack of activity.

> There have been some good discussions recently on re-architecting the device
> feature indicators which were spawned from my OPA MAD changes.

> Various alternatives have been submitted and discussed but Roland has not
> weighed in on which are acceptable.  This makes it difficult to determine what
> direction we should take.

Indeed. No maintainer voice makes it kind of impossible for
discussions to converge. What happens over the last years is that when
there's no easy consensus on matter Y, everyone stops breathing and
wait to see what happens on the rc1 night, b/c Roland doesn't spell
his view/preference (nor exposes his for-next branch till the last
minute, see now) many times it seems more as coin flipping.

> Also, recently I found out my repo for the 0-day build was no longer testing my
> branches because Rolands for-next branch was too old.  I see today that Roland
> has updated to 4.0 rc now.  Thank you.


I added Sagi, Haggai and Shachar from Mellanox. They are behind few of
the major core changes that went in -- Protection/Signature (Sagi),
ODP (all), so if you find some concrete example on why/how these
changes were not architectured well enough, they will be happy to
listen and  respond.

BTW - the Signature verbs are now on their way to new use case, namely
offloading CPU %% used today for CRC and such calculations in
Web2/Hadoop  applications.

Haggai/Shachar are in-charge on the changes for name-spaces to support
containers on which Sean isn't responding for few months.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Roland Dreier]

On Sat, Apr 4, 2015 at 10:15 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Indeed. No maintainer voice makes it kind of impossible for
> discussions to converge. What happens over the last years is that when
> there's no easy consensus on matter Y, everyone stops breathing and
> wait to see what happens on the rc1 night, b/c Roland doesn't spell
> his view/preference (nor exposes his for-next branch till the last
> minute, see now) many times it seems more as coin flipping.

To me this attitude shows a failure of the community.  If I need to
make every decision, then that doesn't scale.  People can ask
questions a lot more easily than I can answer them.

In general if a consensus emerges, I'm pretty likely to trust it.  In
particular, as Sean mentioned, I tend to trust vendors about low-level
drivers, although of course I sometimes catch mistakes even then.

But for changes that touch the core, when there's a disagreement, you
can't expect me to be the one who always solves it.  I might have an
opinion, but in a lot of cases, both sides might have a point and the
only way forward is to come up with a new idea that works for
everyone.  And I'm not smart enough to come up with that solution
every time.

 - R.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[David Miller]

From: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Sun, 5 Apr 2015 07:51:08 -0700

> On Sat, Apr 4, 2015 at 10:15 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Indeed. No maintainer voice makes it kind of impossible for
>> discussions to converge. What happens over the last years is that when
>> there's no easy consensus on matter Y, everyone stops breathing and
>> wait to see what happens on the rc1 night, b/c Roland doesn't spell
>> his view/preference (nor exposes his for-next branch till the last
>> minute, see now) many times it seems more as coin flipping.
> 
> To me this attitude shows a failure of the community.  If I need to
> make every decision, then that doesn't scale.  People can ask
> questions a lot more easily than I can answer them.

No, you need to step in and be the benevolent dictator when people
have their thumbs up their asses and can't make up their minds,
otherwise things grind to a halt.
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Jason Gunthorpe]

On Sun, Apr 05, 2015 at 04:46:26PM -0400, David Miller wrote:
> From: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Date: Sun, 5 Apr 2015 07:51:08 -0700
> 
> > On Sat, Apr 4, 2015 at 10:15 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> Indeed. No maintainer voice makes it kind of impossible for
> >> discussions to converge. What happens over the last years is that when
> >> there's no easy consensus on matter Y, everyone stops breathing and
> >> wait to see what happens on the rc1 night, b/c Roland doesn't spell
> >> his view/preference (nor exposes his for-next branch till the last
> >> minute, see now) many times it seems more as coin flipping.
> > 
> > To me this attitude shows a failure of the community.  If I need to
> > make every decision, then that doesn't scale.  People can ask
> > questions a lot more easily than I can answer them.
> 
> No, you need to step in and be the benevolent dictator when people
> have their thumbs up their asses and can't make up their minds,
> otherwise things grind to a halt.

Agree. 

As a community we don't need the subsystem maintainer to just applies
patches that don't have any discussion. Anyone can do that. We need
someone that guides potentially good patches to success and keeps the
whole code base on track.

I don't think you understand how deep the problem Or is describing
goes.

People have no idea what you want to see, what you will accept and
everytime they ask they get silence.

Or worse - remeber this?

http://www.spinics.net/lists/linux-rdma/msg21114.html

You drive-by-NAK'd Barts's patch and then completely ignored Doug's
follow up, the entire series just died out, and that patch is
still not applied.

This isn't a failure of the community.

There is a pretty clear expectation of what a subsystem maintainer
should do. Greg KH codified some of it in this presentation:

https://github.com/gregkh/presentation-linux-maintainer/blob/master/maintainer.pdf

Start at page 119:

What I will do for you:

 So, finally, you created the perfect patch
 series, took all review into account, and sent
 it correctly, without corrupting the patch.
 What should you expect from me, the
 subsystem maintainer?

Review your patch within 1-2 weeks

 Some subsystem maintainers try to get to
 patches even faster than this, but with travel
 and different conferences, the best that I
 can normally do is about 1-2 weeks.

 If I don't respond in that time frame, just ask
 what is going on. I have no problem with
 people asking about their patch status.
 Sometimes patches end up getting dropped
 on the floor accidentally, and if I'm being
 slow I have no problem with being called on
 it, so don't feel bad about checking up on it.

 But please wait 1-2 weeks, don't be rude and
 send a patch at night, and then in the
 morning send a complaining email asking
 why it wasn't reviewed already. This
 happens more than you want to know.

Offer semi-constructive criticism

Let you know the status of your patch

 I have a set of scripts that I got from Andrew
 Morton that will email you when I apply your
 patch to one of my development trees saying
 where it has been applied and when you can
 expect to see it show up in Linus's tree. There is
 no reason that all kernel maintainers shouldn't do
 this, and it's nice to see that more and more are.

 But, I know from personal experience, there are
 maintainers in this room that I send patches to
 and I never know what happens to them. A few
 months later I will see them show up in Linus's
 tree, usually after I forgot about them.

 That's not acceptable, and you should not allow
 this, push back on your subsystem maintainer to
 use something like this, to keep you informed.
 Andrew's scripts are public, as are my variations
 of them, for everyone to use.

Jason
--
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

Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management

[Or Gerlitz]

On Sun, Apr 5, 2015 at 5:51 PM, Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Sat, Apr 4, 2015 at 10:15 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Indeed. No maintainer voice makes it kind of impossible for
>> discussions to converge. What happens over the last years is that when
>> there's no easy consensus on matter Y, everyone stops breathing and
>> wait to see what happens on the rc1 night, b/c Roland doesn't spell
>> his view/preference (nor exposes his for-next branch till the last
>> minute, see now) many times it seems more as coin flipping.
>
> To me this attitude shows a failure of the community.  If I need to
> make every decision, then that doesn't scale.  People can ask
> questions a lot more easily than I can answer them.
>
> In general if a consensus emerges, I'm pretty likely to trust it.  In
> particular, as Sean mentioned, I tend to trust vendors about low-level
> drivers, although of course I sometimes catch mistakes even then.
>
> But for changes that touch the core, when there's a disagreement, you
> can't expect me to be the one who always solves it.  I might have an
> opinion, but in a lot of cases, both sides might have a point and the
> only way forward is to come up with a new idea that works for
> everyone.  And I'm not smart enough to come up with that solution every time.

To make it a little more clear, my rc1 night note came to describe
situations where
not only that you didn't do the essential part of subsystem maintainer
job of dropping
a note on how you see things and where yo think the solution should be
going etc --
you went ahead and kind of randomly applied patch series for which there was no
consensus or bunch of open reviewer comments unanswered by the
submitters to your
for-next branch which you didn't post publicly and in < 12h sent pull
request to Linus, crazy!!

BTW as we speak now (after rc7), the patches you already have for 4.1
(and there are such, as you acked
@ least one series on the list {BTW about 5y ago you stopped for 90%
of the cases to ack pactches
on the list, which is very frustrating for developers too}) aren't
public, you have a private clone which isn't
reflected at kernel.org

Or.
--
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