| Message ID | 1323205088-29693-1-git-send-email-mchan@broadcom.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Le mardi 06 décembre 2011 à 12:58 -0800, Michael Chan a écrit : > Don't provide FCoE and iSCSI statistics to management firmware if > CONFIG_CNIC is not set. Some needed structure fields are not defined > without CONFIG_CNIC. > > Reported-by: Eric Dumazet <eric.dumazet@gmail.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> > --- > drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > Thanks for the fast answer, and yes, no more build error :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2011-12-06 at 22:03 +0100, Eric Dumazet wrote: > Le mardi 06 décembre 2011 à 12:58 -0800, Michael Chan a écrit : > > Don't provide FCoE and iSCSI statistics to management firmware if > > CONFIG_CNIC is not set. Some needed structure fields are not defined > > without CONFIG_CNIC. > Thanks for the fast answer, and yes, no more build error :) That works, but is that the best solution? Another option is for bnx2x_handle_drv_info_req to return DRV_MSG_CODE_DRV_INFO_NACK Maybe like: switch (op_code) { case ETH_STATS_OPCODE: bnx2x_drv_info_ether_stat(bp); break; #ifdef BCM_CNIC case FCOE_STATS_OPCODE: bnx2x_drv_info_fcoe_stat(bp); break; case ISCSI_STATS_OPCODE: bnx2x_drv_info_iscsi_stat(bp); break; #endif default: /* if op code isn't supported - send NACK */ bnx2x_fw_command(bp, DRV_MSG_CODE_DRV_INFO_NACK, 0); return; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2011-12-06 at 13:25 -0800, Joe Perches wrote: > On Tue, 2011-12-06 at 22:03 +0100, Eric Dumazet wrote: > > Le mardi 06 décembre 2011 à 12:58 -0800, Michael Chan a écrit : > > > Don't provide FCoE and iSCSI statistics to management firmware if > > > CONFIG_CNIC is not set. Some needed structure fields are not defined > > > without CONFIG_CNIC. > > Thanks for the fast answer, and yes, no more build error :) > > That works, but is that the best solution? > > Another option is for bnx2x_handle_drv_info_req > to return DRV_MSG_CODE_DRV_INFO_NACK > Eilon (bnx2x lead maintainer) will need to decide which is the most appropriate solution as he is most familiar with the firmware. I don't know if sending NACK to the firmware will have other negative effects or not. Eilon should be online in about 8 hours and he can send a follow-up patch if necessary. > Maybe like: > > switch (op_code) { > case ETH_STATS_OPCODE: > bnx2x_drv_info_ether_stat(bp); > break; > #ifdef BCM_CNIC > case FCOE_STATS_OPCODE: > bnx2x_drv_info_fcoe_stat(bp); > break; > case ISCSI_STATS_OPCODE: > bnx2x_drv_info_iscsi_stat(bp); > break; > #endif > default: > /* if op code isn't supported - send NACK */ > bnx2x_fw_command(bp, DRV_MSG_CODE_DRV_INFO_NACK, 0); > return; > } > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 06 Dec 2011 22:03:03 +0100 > Le mardi 06 décembre 2011 à 12:58 -0800, Michael Chan a écrit : >> Don't provide FCoE and iSCSI statistics to management firmware if >> CONFIG_CNIC is not set. Some needed structure fields are not defined >> without CONFIG_CNIC. >> >> Reported-by: Eric Dumazet <eric.dumazet@gmail.com> >> Signed-off-by: Michael Chan <mchan@broadcom.com> >> --- >> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> > > Thanks for the fast answer, and yes, no more build error :) Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2011-12-06 at 13:42 -0800, Michael Chan wrote: > On Tue, 2011-12-06 at 13:25 -0800, Joe Perches wrote: > > On Tue, 2011-12-06 at 22:03 +0100, Eric Dumazet wrote: > > > Le mardi 06 décembre 2011 à 12:58 -0800, Michael Chan a écrit : > > > > Don't provide FCoE and iSCSI statistics to management firmware if > > > > CONFIG_CNIC is not set. Some needed structure fields are not defined > > > > without CONFIG_CNIC. > > > Thanks for the fast answer, and yes, no more build error :) > > > > That works, but is that the best solution? > > > > Another option is for bnx2x_handle_drv_info_req > > to return DRV_MSG_CODE_DRV_INFO_NACK > > > > Eilon (bnx2x lead maintainer) will need to decide which is the most > appropriate solution as he is most familiar with the firmware. I don't > know if sending NACK to the firmware will have other negative effects or > not. Eilon should be online in about 8 hours and he can send a > follow-up patch if necessary. > > > Maybe like: > > > > switch (op_code) { > > case ETH_STATS_OPCODE: > > bnx2x_drv_info_ether_stat(bp); > > break; > > #ifdef BCM_CNIC > > case FCOE_STATS_OPCODE: > > bnx2x_drv_info_fcoe_stat(bp); > > break; > > case ISCSI_STATS_OPCODE: > > bnx2x_drv_info_iscsi_stat(bp); > > break; > > #endif > > default: > > /* if op code isn't supported - send NACK */ > > bnx2x_fw_command(bp, DRV_MSG_CODE_DRV_INFO_NACK, 0); > > return; > > } > > > > > > > Joe is right. This latest patch breaks the FW assumption, but at least everything compiles now, so it is not as urgent. We will send a patch in that spirit in the morning. Thanks, Eilon -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2011-12-07 at 00:16 +0200, Eilon Greenstein wrote: > On Tue, 2011-12-06 at 13:42 -0800, Michael Chan wrote: > > On Tue, 2011-12-06 at 13:25 -0800, Joe Perches wrote: > > > On Tue, 2011-12-06 at 22:03 +0100, Eric Dumazet wrote: > > > > Le mardi 06 décembre 2011 à 12:58 -0800, Michael Chan a écrit : > > > > > Don't provide FCoE and iSCSI statistics to management firmware if > > > > > CONFIG_CNIC is not set. Some needed structure fields are not defined > > > > > without CONFIG_CNIC. > > > > Thanks for the fast answer, and yes, no more build error :) > > > > > > That works, but is that the best solution? > > > > > > Another option is for bnx2x_handle_drv_info_req > > > to return DRV_MSG_CODE_DRV_INFO_NACK > > > > > > > Eilon (bnx2x lead maintainer) will need to decide which is the most > > appropriate solution as he is most familiar with the firmware. I don't > > know if sending NACK to the firmware will have other negative effects or > > not. Eilon should be online in about 8 hours and he can send a > > follow-up patch if necessary. > > > > > Maybe like: > > > > > > switch (op_code) { > > > case ETH_STATS_OPCODE: > > > bnx2x_drv_info_ether_stat(bp); > > > break; > > > #ifdef BCM_CNIC > > > case FCOE_STATS_OPCODE: > > > bnx2x_drv_info_fcoe_stat(bp); > > > break; > > > case ISCSI_STATS_OPCODE: > > > bnx2x_drv_info_iscsi_stat(bp); > > > break; > > > #endif > > > default: > > > /* if op code isn't supported - send NACK */ > > > bnx2x_fw_command(bp, DRV_MSG_CODE_DRV_INFO_NACK, 0); > > > return; > > > } > > > > > > > > > > > > > Joe is right. This latest patch breaks the FW assumption, but at least > everything compiles now, so it is not as urgent. We will send a patch in > that spirit in the morning. As you probably saw in the patch Barak just sent, after thinking about it some more and testing the FW flows, it is best to return ACK without updating the counters. Eilon -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index 86d36f8..418e7d3 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c @@ -2943,6 +2943,7 @@ static void bnx2x_drv_info_ether_stat(struct bnx2x *bp) static void bnx2x_drv_info_fcoe_stat(struct bnx2x *bp) { +#ifdef BCM_CNIC struct bnx2x_dcbx_app_params *app = &bp->dcbx_port_params.app; struct fcoe_stats_info *fcoe_stat = &bp->slowpath->drv_info_to_mcp.fcoe_stat; @@ -3026,7 +3027,6 @@ static void bnx2x_drv_info_fcoe_stat(struct bnx2x *bp) fcoe_q_xstorm_stats->mcast_pkts_sent); } -#ifdef BCM_CNIC /* ask L5 driver to add data to the struct */ bnx2x_cnic_notify(bp, CNIC_CTL_FCOE_STATS_GET_CMD); #endif @@ -3034,6 +3034,7 @@ static void bnx2x_drv_info_fcoe_stat(struct bnx2x *bp) static void bnx2x_drv_info_iscsi_stat(struct bnx2x *bp) { +#ifdef BCM_CNIC struct bnx2x_dcbx_app_params *app = &bp->dcbx_port_params.app; struct iscsi_stats_info *iscsi_stat = &bp->slowpath->drv_info_to_mcp.iscsi_stat; @@ -3043,7 +3044,6 @@ static void bnx2x_drv_info_iscsi_stat(struct bnx2x *bp) iscsi_stat->qos_priority = app->traffic_type_priority[LLFC_TRAFFIC_TYPE_ISCSI]; -#ifdef BCM_CNIC /* ask L5 driver to add data to the struct */ bnx2x_cnic_notify(bp, CNIC_CTL_ISCSI_STATS_GET_CMD); #endif
Don't provide FCoE and iSCSI statistics to management firmware if CONFIG_CNIC is not set. Some needed structure fields are not defined without CONFIG_CNIC. Reported-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: Michael Chan <mchan@broadcom.com> --- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)