* [PATCH for-next 0/9] mlx4 changes in virtual GID management @ 2015-03-29 13:51 Or Gerlitz [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]
* [PATCH for-next 1/9] IB/mlx4: Alias GUID adding persistency support [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 2/9] net/mlx4_core: Manage alias GUID per VF Or Gerlitz ` (8 subsequent siblings) 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH for-next 2/9] net/mlx4_core: Manage alias GUID per VF [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2015-03-29 13:51 ` [PATCH for-next 1/9] IB/mlx4: Alias GUID adding persistency support Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs Or Gerlitz ` (7 subsequent siblings) 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2015-03-29 13:51 ` [PATCH for-next 1/9] IB/mlx4: Alias GUID adding persistency support Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 2/9] net/mlx4_core: Manage alias GUID per VF Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz [not found] ` <1427637093-6711-4-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2015-03-29 13:51 ` [PATCH for-next 4/9] IB/mlx4: Manage admin alias GUID upon admin request Or Gerlitz ` (6 subsequent siblings) 9 siblings, 1 reply; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
[parent not found: <1427637093-6711-4-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs [not found] ` <1427637093-6711-4-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> @ 2015-03-30 17:16 ` Jason Gunthorpe [not found] ` <20150330171631.GA1152-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Jason Gunthorpe @ 2015-03-30 17:16 UTC (permalink / raw) To: Or Gerlitz Cc: Roland Dreier, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <20150330171631.GA1152-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs [not found] ` <20150330171631.GA1152-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2015-03-31 9:54 ` Or Gerlitz 0 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-31 9:54 UTC (permalink / raw) To: Jason Gunthorpe Cc: Roland Dreier, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH for-next 4/9] IB/mlx4: Manage admin alias GUID upon admin request [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> ` (2 preceding siblings ...) 2015-03-29 13:51 ` [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 5/9] IB/mlx4: Change init flow to request alias GUIDs for active VFs Or Gerlitz ` (5 subsequent siblings) 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH for-next 5/9] IB/mlx4: Change init flow to request alias GUIDs for active VFs [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> ` (3 preceding siblings ...) 2015-03-29 13:51 ` [PATCH for-next 4/9] IB/mlx4: Manage admin alias GUID upon admin request Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 6/9] IB/mlx4: Request alias GUID on demand Or Gerlitz ` (4 subsequent siblings) 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH for-next 6/9] IB/mlx4: Request alias GUID on demand [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> ` (4 preceding siblings ...) 2015-03-29 13:51 ` [PATCH for-next 5/9] IB/mlx4: Change init flow to request alias GUIDs for active VFs Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 7/9] net/mlx4_core: Raise slave shutdown event upon FLR Or Gerlitz ` (3 subsequent siblings) 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH for-next 7/9] net/mlx4_core: Raise slave shutdown event upon FLR [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> ` (5 preceding siblings ...) 2015-03-29 13:51 ` [PATCH for-next 6/9] IB/mlx4: Request alias GUID on demand Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 8/9] net/mlx4_core: Return the admin alias GUID upon host view request Or Gerlitz ` (2 subsequent siblings) 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH for-next 8/9] net/mlx4_core: Return the admin alias GUID upon host view request [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> ` (6 preceding siblings ...) 2015-03-29 13:51 ` [PATCH for-next 7/9] net/mlx4_core: Raise slave shutdown event upon FLR Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 9/9] IB/mlx4: Change alias guids default to be host assigned Or Gerlitz 2015-03-30 16:17 ` [PATCH for-next 0/9] mlx4 changes in virtual GID management Or Gerlitz 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH for-next 9/9] IB/mlx4: Change alias guids default to be host assigned [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> ` (7 preceding siblings ...) 2015-03-29 13:51 ` [PATCH for-next 8/9] net/mlx4_core: Return the admin alias GUID upon host view request Or Gerlitz @ 2015-03-29 13:51 ` Or Gerlitz 2015-03-30 16:17 ` [PATCH for-next 0/9] mlx4 changes in virtual GID management Or Gerlitz 9 siblings, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-03-29 13:51 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Amir Vadai, Yishai Hadas, Jack Morgenstein, 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> ` (8 preceding siblings ...) 2015-03-29 13:51 ` [PATCH for-next 9/9] IB/mlx4: Change alias guids default to be host assigned Or Gerlitz @ 2015-03-30 16:17 ` Or Gerlitz [not found] ` <CAJ3xEMj0T8QXBQdVHmfEFMXwjAFVD-O6ywAwyyVY+M3oRLzAVA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 9 siblings, 1 reply; 34+ messages in thread From: Or Gerlitz @ 2015-03-30 16:17 UTC (permalink / raw) To: Roland Dreier Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Tal Alon, Roland Dreier, David Miller, Amir Vadai 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAJ3xEMj0T8QXBQdVHmfEFMXwjAFVD-O6ywAwyyVY+M3oRLzAVA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAJ3xEMj0T8QXBQdVHmfEFMXwjAFVD-O6ywAwyyVY+M3oRLzAVA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-03-31 3:36 ` David Miller [not found] ` <20150330.233602.155832546277570456.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: David Miller @ 2015-03-31 3:36 UTC (permalink / raw) To: gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w Cc: roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, roland-BHEL68pLQRGGvPXPguhicg, amirv-VPRAkNaXOzVWk0Htik3J/w 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <20150330.233602.155832546277570456.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <20150330.233602.155832546277570456.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> @ 2015-03-31 3:47 ` Roland Dreier [not found] ` <CAL1RGDUmDGCGdTBeTTBiHygO5UgEbRm_Qgxtx-+bxo1vg1v-8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Roland Dreier @ 2015-03-31 3:47 UTC (permalink / raw) To: David Miller Cc: Or Gerlitz, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, Amir Vadai > 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAL1RGDUmDGCGdTBeTTBiHygO5UgEbRm_Qgxtx-+bxo1vg1v-8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAL1RGDUmDGCGdTBeTTBiHygO5UgEbRm_Qgxtx-+bxo1vg1v-8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-03-31 9:13 ` Sagi Grimberg [not found] ` <551A6556.9030708-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-03-31 11:22 ` Or Gerlitz 1 sibling, 1 reply; 34+ messages in thread From: Sagi Grimberg @ 2015-03-31 9:13 UTC (permalink / raw) To: Roland Dreier, David Miller Cc: Or Gerlitz, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, Amir Vadai 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <551A6556.9030708-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <551A6556.9030708-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> @ 2015-03-31 18:46 ` Jason Gunthorpe 0 siblings, 0 replies; 34+ messages in thread From: Jason Gunthorpe @ 2015-03-31 18:46 UTC (permalink / raw) To: Sagi Grimberg Cc: Roland Dreier, David Miller, Or Gerlitz, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, Amir Vadai 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAL1RGDUmDGCGdTBeTTBiHygO5UgEbRm_Qgxtx-+bxo1vg1v-8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-03-31 9:13 ` Sagi Grimberg @ 2015-03-31 11:22 ` Or Gerlitz [not found] ` <CAJ3xEMjfbxt2Ouh4bhuf3_LMc7qY807h6FryvdCr0rr_gdiZAw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 34+ messages in thread From: Or Gerlitz @ 2015-03-31 11:22 UTC (permalink / raw) To: Roland Dreier Cc: David Miller, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, Amir Vadai 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAJ3xEMjfbxt2Ouh4bhuf3_LMc7qY807h6FryvdCr0rr_gdiZAw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAJ3xEMjfbxt2Ouh4bhuf3_LMc7qY807h6FryvdCr0rr_gdiZAw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-03-31 12:49 ` Christoph Lameter [not found] ` <alpine.DEB.2.11.1503310735380.13128-gkYfJU5Cukgdnm+yROfE0A@public.gmane.org> 2015-03-31 15:48 ` David Miller 1 sibling, 1 reply; 34+ messages in thread From: Christoph Lameter @ 2015-03-31 12:49 UTC (permalink / raw) To: Or Gerlitz Cc: Roland Dreier, David Miller, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Amir Vadai, hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <alpine.DEB.2.11.1503310735380.13128-gkYfJU5Cukgdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <alpine.DEB.2.11.1503310735380.13128-gkYfJU5Cukgdnm+yROfE0A@public.gmane.org> @ 2015-03-31 12:57 ` Hal Rosenstock [not found] ` <551A99D4.7060703-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-03-31 15:50 ` David Miller 1 sibling, 1 reply; 34+ messages in thread From: Hal Rosenstock @ 2015-03-31 12:57 UTC (permalink / raw) To: Christoph Lameter Cc: Or Gerlitz, Roland Dreier, David Miller, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Amir Vadai 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <551A99D4.7060703-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <551A99D4.7060703-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> @ 2015-03-31 15:49 ` Christoph Lameter 0 siblings, 0 replies; 34+ messages in thread From: Christoph Lameter @ 2015-03-31 15:49 UTC (permalink / raw) To: Hal Rosenstock Cc: Or Gerlitz, Roland Dreier, David Miller, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Amir Vadai 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <alpine.DEB.2.11.1503310735380.13128-gkYfJU5Cukgdnm+yROfE0A@public.gmane.org> 2015-03-31 12:57 ` Hal Rosenstock @ 2015-03-31 15:50 ` David Miller [not found] ` <20150331.115052.1321302787804579694.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 1 sibling, 1 reply; 34+ messages in thread From: David Miller @ 2015-03-31 15:50 UTC (permalink / raw) To: cl-vYTEC60ixJUAvxtiuMwx3w Cc: gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w, roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, amirv-VPRAkNaXOzVWk0Htik3J/w, hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <20150331.115052.1321302787804579694.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <20150331.115052.1321302787804579694.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> @ 2015-03-31 16:29 ` Christoph Lameter 0 siblings, 0 replies; 34+ messages in thread From: Christoph Lameter @ 2015-03-31 16:29 UTC (permalink / raw) To: David Miller Cc: gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w, roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, amirv-VPRAkNaXOzVWk0Htik3J/w, hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAJ3xEMjfbxt2Ouh4bhuf3_LMc7qY807h6FryvdCr0rr_gdiZAw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-03-31 12:49 ` Christoph Lameter @ 2015-03-31 15:48 ` David Miller [not found] ` <20150331.114824.651005354305268415.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 1 sibling, 1 reply; 34+ messages in thread From: David Miller @ 2015-03-31 15:48 UTC (permalink / raw) To: gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w Cc: roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <20150331.114824.651005354305268415.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>]
* RE: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <20150331.114824.651005354305268415.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> @ 2015-03-31 17:27 ` Hefty, Sean [not found] ` <1828884A29C6694DAF28B7E6B8A82373A8FBB790-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Hefty, Sean @ 2015-03-31 17:27 UTC (permalink / raw) To: David Miller, gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w Cc: roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w > 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <1828884A29C6694DAF28B7E6B8A82373A8FBB790-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <1828884A29C6694DAF28B7E6B8A82373A8FBB790-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org> @ 2015-03-31 20:28 ` Or Gerlitz 2015-03-31 21:33 ` Hefty, Sean 0 siblings, 1 reply; 34+ messages in thread From: Or Gerlitz @ 2015-03-31 20:28 UTC (permalink / raw) To: Hefty, Sean Cc: David Miller, roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCH for-next 0/9] mlx4 changes in virtual GID management 2015-03-31 20:28 ` Or Gerlitz @ 2015-03-31 21:33 ` Hefty, Sean [not found] ` <1828884A29C6694DAF28B7E6B8A82373A8FBBCE3-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Hefty, Sean @ 2015-03-31 21:33 UTC (permalink / raw) To: Or Gerlitz Cc: David Miller, roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w > 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <1828884A29C6694DAF28B7E6B8A82373A8FBBCE3-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <1828884A29C6694DAF28B7E6B8A82373A8FBBCE3-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org> @ 2015-04-02 14:32 ` Or Gerlitz [not found] ` <CAJ3xEMgptECgnWXfW7wN8sjqRfUvzF3tCN=Lj8MZtdOG8yg3jQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Or Gerlitz @ 2015-04-02 14:32 UTC (permalink / raw) To: Hefty, Sean Cc: David Miller, roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAJ3xEMgptECgnWXfW7wN8sjqRfUvzF3tCN=Lj8MZtdOG8yg3jQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAJ3xEMgptECgnWXfW7wN8sjqRfUvzF3tCN=Lj8MZtdOG8yg3jQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-04-02 22:31 ` ira.weiny [not found] ` <20150402223135.GA12588-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: ira.weiny @ 2015-04-02 22:31 UTC (permalink / raw) To: Or Gerlitz Cc: Hefty, Sean, David Miller, roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <20150402223135.GA12588-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <20150402223135.GA12588-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> @ 2015-04-05 5:15 ` Or Gerlitz [not found] ` <CAJ3xEMgsHAFJomuCN+EzMcYaxTOTQqHuDdr9zztuO9pH9QicXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Or Gerlitz @ 2015-04-05 5:15 UTC (permalink / raw) To: ira.weiny Cc: Hefty, Sean, David Miller, roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w, Sagi Grimberg, Haggai Eran, Shachar Raindel 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAJ3xEMgsHAFJomuCN+EzMcYaxTOTQqHuDdr9zztuO9pH9QicXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAJ3xEMgsHAFJomuCN+EzMcYaxTOTQqHuDdr9zztuO9pH9QicXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-04-05 14:51 ` Roland Dreier [not found] ` <CAG4TOxOv98YjNOi9MbKHiHg6aLw75XRfasMaScdRccRitiE3-g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 34+ messages in thread From: Roland Dreier @ 2015-04-05 14:51 UTC (permalink / raw) To: Or Gerlitz Cc: ira.weiny, Hefty, Sean, David Miller, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w, Sagi Grimberg, Haggai Eran, Shachar Raindel 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CAG4TOxOv98YjNOi9MbKHiHg6aLw75XRfasMaScdRccRitiE3-g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAG4TOxOv98YjNOi9MbKHiHg6aLw75XRfasMaScdRccRitiE3-g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-04-05 20:46 ` David Miller [not found] ` <20150405.164626.1878934248335902055.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 2015-04-08 13:03 ` Or Gerlitz 1 sibling, 1 reply; 34+ messages in thread From: David Miller @ 2015-04-05 20:46 UTC (permalink / raw) To: roland-DgEjT+Ai2ygdnm+yROfE0A Cc: gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w, sagig-VPRAkNaXOzVWk0Htik3J/w, haggaie-VPRAkNaXOzVWk0Htik3J/w, raindel-VPRAkNaXOzVWk0Htik3J/w 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <20150405.164626.1878934248335902055.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>]
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <20150405.164626.1878934248335902055.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> @ 2015-04-07 17:12 ` Jason Gunthorpe 0 siblings, 0 replies; 34+ messages in thread From: Jason Gunthorpe @ 2015-04-07 17:12 UTC (permalink / raw) To: David Miller Cc: roland-DgEjT+Ai2ygdnm+yROfE0A, gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w, sagig-VPRAkNaXOzVWk0Htik3J/w, haggaie-VPRAkNaXOzVWk0Htik3J/w, raindel-VPRAkNaXOzVWk0Htik3J/w 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH for-next 0/9] mlx4 changes in virtual GID management [not found] ` <CAG4TOxOv98YjNOi9MbKHiHg6aLw75XRfasMaScdRccRitiE3-g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-04-05 20:46 ` David Miller @ 2015-04-08 13:03 ` Or Gerlitz 1 sibling, 0 replies; 34+ messages in thread From: Or Gerlitz @ 2015-04-08 13:03 UTC (permalink / raw) To: Roland Dreier Cc: ira.weiny, Hefty, Sean, David Miller, linux-rdma-u79uwXL29TY76Z2rM5mHXA, talal-VPRAkNaXOzVWk0Htik3J/w, amirv-VPRAkNaXOzVWk0Htik3J/w, Sagi Grimberg, Haggai Eran, Shachar Raindel 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2015-04-08 13:03 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-29 13:51 [PATCH for-next 0/9] mlx4 changes in virtual GID management Or Gerlitz [not found] ` <1427637093-6711-1-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2015-03-29 13:51 ` [PATCH for-next 1/9] IB/mlx4: Alias GUID adding persistency support Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 2/9] net/mlx4_core: Manage alias GUID per VF Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 3/9] net/mlx4_core: Set initial admin GUIDs for VFs Or Gerlitz [not found] ` <1427637093-6711-4-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2015-03-30 17:16 ` Jason Gunthorpe [not found] ` <20150330171631.GA1152-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2015-03-31 9:54 ` Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 4/9] IB/mlx4: Manage admin alias GUID upon admin request Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 5/9] IB/mlx4: Change init flow to request alias GUIDs for active VFs Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 6/9] IB/mlx4: Request alias GUID on demand Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 7/9] net/mlx4_core: Raise slave shutdown event upon FLR Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 8/9] net/mlx4_core: Return the admin alias GUID upon host view request Or Gerlitz 2015-03-29 13:51 ` [PATCH for-next 9/9] IB/mlx4: Change alias guids default to be host assigned Or Gerlitz 2015-03-30 16:17 ` [PATCH for-next 0/9] mlx4 changes in virtual GID management Or Gerlitz [not found] ` <CAJ3xEMj0T8QXBQdVHmfEFMXwjAFVD-O6ywAwyyVY+M3oRLzAVA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-03-31 3:36 ` David Miller [not found] ` <20150330.233602.155832546277570456.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 2015-03-31 3:47 ` Roland Dreier [not found] ` <CAL1RGDUmDGCGdTBeTTBiHygO5UgEbRm_Qgxtx-+bxo1vg1v-8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-03-31 9:13 ` Sagi Grimberg [not found] ` <551A6556.9030708-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-03-31 18:46 ` Jason Gunthorpe 2015-03-31 11:22 ` Or Gerlitz [not found] ` <CAJ3xEMjfbxt2Ouh4bhuf3_LMc7qY807h6FryvdCr0rr_gdiZAw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-03-31 12:49 ` Christoph Lameter [not found] ` <alpine.DEB.2.11.1503310735380.13128-gkYfJU5Cukgdnm+yROfE0A@public.gmane.org> 2015-03-31 12:57 ` Hal Rosenstock [not found] ` <551A99D4.7060703-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-03-31 15:49 ` Christoph Lameter 2015-03-31 15:50 ` David Miller [not found] ` <20150331.115052.1321302787804579694.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 2015-03-31 16:29 ` Christoph Lameter 2015-03-31 15:48 ` David Miller [not found] ` <20150331.114824.651005354305268415.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 2015-03-31 17:27 ` Hefty, Sean [not found] ` <1828884A29C6694DAF28B7E6B8A82373A8FBB790-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org> 2015-03-31 20:28 ` Or Gerlitz 2015-03-31 21:33 ` Hefty, Sean [not found] ` <1828884A29C6694DAF28B7E6B8A82373A8FBBCE3-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org> 2015-04-02 14:32 ` Or Gerlitz [not found] ` <CAJ3xEMgptECgnWXfW7wN8sjqRfUvzF3tCN=Lj8MZtdOG8yg3jQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-04-02 22:31 ` ira.weiny [not found] ` <20150402223135.GA12588-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2015-04-05 5:15 ` Or Gerlitz [not found] ` <CAJ3xEMgsHAFJomuCN+EzMcYaxTOTQqHuDdr9zztuO9pH9QicXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-04-05 14:51 ` Roland Dreier [not found] ` <CAG4TOxOv98YjNOi9MbKHiHg6aLw75XRfasMaScdRccRitiE3-g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-04-05 20:46 ` David Miller [not found] ` <20150405.164626.1878934248335902055.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> 2015-04-07 17:12 ` Jason Gunthorpe 2015-04-08 13:03 ` Or Gerlitz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.