[PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[Michael Chan]

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

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

Re: [PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[Eric Dumazet]

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

Re: [PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[Joe Perches]

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

Re: [PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[Michael Chan]

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

Re: [PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[David Miller]

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.

Re: [PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[Eilon Greenstein]

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

Re: [PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set

[Eilon Greenstein]

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