RE: [PATCH] net: broadcom: bnx2x: make a couple of const arrays static

> Don't populate various tables on the stack but make them static const. > Makes the object code smaller by nearly 200 bytes: > > Before: >text data bss dec hex filename > 113468 11200 0 124668 1e6fc bnx2x_ethtool.o > > After: >text data

RE: [PATCH] net: broadcom: bnx2x: make a couple of const arrays static

> Don't populate various tables on the stack but make them static const. > Makes the object code smaller by nearly 200 bytes: > > Before: >text data bss dec hex filename > 113468 11200 0 124668 1e6fc bnx2x_ethtool.o > > After: >text data

RE: [PATCH net-next 1/9] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC

> > > +static void hns3_nic_net_down(struct net_device *ndev) { > > > + struct hns3_nic_priv *priv = netdev_priv(ndev); > > > + struct hnae3_ae_ops *ops; > > > + int i; > > > + > > > + netif_tx_stop_all_queues(ndev); > > > + netif_carrier_off(ndev); > > > + netif_tx_disable(ndev); > > > + > > > +

RE: [PATCH net-next 1/9] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC

> > > +static void hns3_nic_net_down(struct net_device *ndev) { > > > + struct hns3_nic_priv *priv = netdev_priv(ndev); > > > + struct hnae3_ae_ops *ops; > > > + int i; > > > + > > > + netif_tx_stop_all_queues(ndev); > > > + netif_carrier_off(ndev); > > > + netif_tx_disable(ndev); > > > + > > > +

RE: [PATCH net-next 8/9] net: hns3: Add support of debugfs interface to HNS3 driver

> This adds the support of the debugfs interface to the driver for debugging > purposes. > +const struct hclge_support_cmd support_cmd[] = { > + {"send cmd", 8, hclge_dbg_send, > + "opcode flag data0 data1 data2 data3 data4 data5"}, > + {"help", 4, hclge_dbg_usage, "no

RE: [PATCH net-next 8/9] net: hns3: Add support of debugfs interface to HNS3 driver

> This adds the support of the debugfs interface to the driver for debugging > purposes. > +const struct hclge_support_cmd support_cmd[] = { > + {"send cmd", 8, hclge_dbg_send, > + "opcode flag data0 data1 data2 data3 data4 data5"}, > + {"help", 4, hclge_dbg_usage, "no

RE: [PATCH net-next 1/9] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC

> +static void hns3_nic_net_down(struct net_device *ndev) { > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_ae_ops *ops; > + int i; > + > + netif_tx_stop_all_queues(ndev); > + netif_carrier_off(ndev); > + netif_tx_disable(ndev); > + > + ops =

RE: [PATCH net-next 1/9] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC

> +static void hns3_nic_net_down(struct net_device *ndev) { > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_ae_ops *ops; > + int i; > + > + netif_tx_stop_all_queues(ndev); > + netif_carrier_off(ndev); > + netif_tx_disable(ndev); > + > + ops =

RE: [PATCH net-next] qed: add qed_int_sb_init() stub function

> When CONFIG_QED_SRIOV is disabled, we get a build error: > > drivers/net/ethernet/qlogic/qed/qed_int.c: In function 'qed_int_sb_init': > drivers/net/ethernet/qlogic/qed/qed_int.c:1499:4: error: implicit declaration > of function 'qed_vf_set_sb_info'; did you mean 'qed_mcp_get_resc_info'? [- >

RE: [PATCH net-next] qed: add qed_int_sb_init() stub function

> When CONFIG_QED_SRIOV is disabled, we get a build error: > > drivers/net/ethernet/qlogic/qed/qed_int.c: In function 'qed_int_sb_init': > drivers/net/ethernet/qlogic/qed/qed_int.c:1499:4: error: implicit declaration > of function 'qed_vf_set_sb_info'; did you mean 'qed_mcp_get_resc_info'? [- >

RE: [PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc

> The driver may sleep in interrupt handling, and the function call path is: > qed_int_sp_dpc (tasklet_init indicates it handles interrupt) > qed_int_attentions > qed_mcp_handle_events > qed_mcp_handle_link_change > qed_link_update > qed_fill_link >

RE: [PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc

> The driver may sleep in interrupt handling, and the function call path is: > qed_int_sp_dpc (tasklet_init indicates it handles interrupt) > qed_int_attentions > qed_mcp_handle_events > qed_mcp_handle_link_change > qed_link_update > qed_fill_link >

RE: [net-qed] question about potential null pointer dereference

> While looking into Coverity ID 1362293 I ran into the following piece of code > at drivers/net/ethernet/qlogic/qed/qed_sriov.c:3863: > > 3863static int > 3864qed_iov_configure_min_tx_rate(struct qed_dev *cdev, int vfid, u32 rate) > 3865{ > 3866struct qed_vf_info *vf; > 3867u8

RE: [net-qed] question about potential null pointer dereference

> While looking into Coverity ID 1362293 I ran into the following piece of code > at drivers/net/ethernet/qlogic/qed/qed_sriov.c:3863: > > 3863static int > 3864qed_iov_configure_min_tx_rate(struct qed_dev *cdev, int vfid, u32 rate) > 3865{ > 3866struct qed_vf_info *vf; > 3867u8

RE: [PATCH] qed: fix uninitialized data in aRFS intrastructure

> > For the most part - I'm almost all in favor of this change. > > But just to make it clear - the actual fix could have been a one-liner, > > right? > > The rest are style changes. > Correct. Having the correct length in the memset is a sufficient fix for the > warning, > but it felt wrong to

RE: [PATCH] qed: fix uninitialized data in aRFS intrastructure

> > For the most part - I'm almost all in favor of this change. > > But just to make it clear - the actual fix could have been a one-liner, > > right? > > The rest are style changes. > Correct. Having the correct length in the memset is a sufficient fix for the > warning, > but it felt wrong to

RE: [PATCH] qed: fix uninitialized data in aRFS intrastructure

> register, which went subtly wrong due to the wrong size in a memset(): > > ethernet/qlogic/qed/qed_init_fw_funcs.c: In function > 'qed_set_rfs_mode_disable': > ethernet/qlogic/qed/qed_init_fw_funcs.c:993:3: error: '*((void > *)+4)' is used uninitialized in this function [-Werror=uninitialized]

RE: [PATCH] qed: fix uninitialized data in aRFS intrastructure

> register, which went subtly wrong due to the wrong size in a memset(): > > ethernet/qlogic/qed/qed_init_fw_funcs.c: In function > 'qed_set_rfs_mode_disable': > ethernet/qlogic/qed/qed_init_fw_funcs.c:993:3: error: '*((void > *)+4)' is used uninitialized in this function [-Werror=uninitialized]

RE: [PATCH] qed: fix missing break in OOO_LB_TC case

> There seems to be a missing break on the OOO_LB_TC case, pq_id is being > assigned and then re-assigned on the fall through default case and that > seems suspect. > > Detected by CoverityScan, CID#1424402 ("Missing break in switch") > > Fixes: b5a9ee7cf3be1 ("qed: Revise QM cofiguration") > >

RE: [PATCH] qed: fix missing break in OOO_LB_TC case

> There seems to be a missing break on the OOO_LB_TC case, pq_id is being > assigned and then re-assigned on the fall through default case and that > seems suspect. > > Detected by CoverityScan, CID#1424402 ("Missing break in switch") > > Fixes: b5a9ee7cf3be1 ("qed: Revise QM cofiguration") > >

RE: [PATCH] bnx2x: fix spelling mistake in macros HW_INTERRUT_ASSERT_SET_*

> Trival fix, rename HW_INTERRUT_ASSERT_SET_* to > HW_INTERRUPT_ASSERT_SET_* > > Signed-off-by: Colin Ian King Thanks. Don't know if it's needed but still: Acked-by: Yuval Mintz

RE: [PATCH] bnx2x: fix spelling mistake in macros HW_INTERRUT_ASSERT_SET_*

> Trival fix, rename HW_INTERRUT_ASSERT_SET_* to > HW_INTERRUPT_ASSERT_SET_* > > Signed-off-by: Colin Ian King Thanks. Don't know if it's needed but still: Acked-by: Yuval Mintz

RE: [PATCH v2] net: broadcom: bnx2x: use new api ethtool_{get|set}_link_ksettings

> The ethtool api {get|set}_settings is deprecated. > We move this driver to new api {get|set}_link_ksettings. > > As I don't have the hardware, I'd be very pleased if someone may test this > patch. > > Signed-off-by: Philippe Reynes >From the little testing I've did, things

RE: [PATCH v2] net: broadcom: bnx2x: use new api ethtool_{get|set}_link_ksettings

> The ethtool api {get|set}_settings is deprecated. > We move this driver to new api {get|set}_link_ksettings. > > As I don't have the hardware, I'd be very pleased if someone may test this > patch. > > Signed-off-by: Philippe Reynes >From the little testing I've did, things look fine. Thanks.

RE: [PATCH 2/3] qed: avoid possible stack overflow in qed_ll2_acquire_connection

> 968,23 +968,19 @@ static int qed_ll2_start_ooo(struct qed_dev *cdev, { > struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev); > u8 *handle = >pf_params.iscsi_pf_params.ll2_ooo_queue_id; > - struct qed_ll2_info *ll2_info; > + struct qed_ll2_conn ll2_info; A bit confusing to

RE: [PATCH 2/3] qed: avoid possible stack overflow in qed_ll2_acquire_connection

> 968,23 +968,19 @@ static int qed_ll2_start_ooo(struct qed_dev *cdev, { > struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev); > u8 *handle = >pf_params.iscsi_pf_params.ll2_ooo_queue_id; > - struct qed_ll2_info *ll2_info; > + struct qed_ll2_conn ll2_info; A bit confusing to

RE: [PATCH][V2] qed: fix memory leak of a qed_spq_entry on error failure paths

> From: Colin Ian King > > A qed_spq_entry entry is allocated by qed_sp_init_request but is not kfree'd > if an error occurs, causing a memory leak. Fix this by returning the > previously > allocated spq entry and also setting *pp_ent to NULL to be safe. > > Thanks to

RE: [PATCH][V2] qed: fix memory leak of a qed_spq_entry on error failure paths

> From: Colin Ian King > > A qed_spq_entry entry is allocated by qed_sp_init_request but is not kfree'd > if an error occurs, causing a memory leak. Fix this by returning the > previously > allocated spq entry and also setting *pp_ent to NULL to be safe. > > Thanks to Yuval Mintz for

RE: [PATCH] qed: fix memory leak of a qed_spq_entry on error failure paths

> From: Colin Ian King > > A qed_spq_entry entry is allocated by qed_sp_init_request but is not kfree'd > if an error occurs, causing a memory leak. Fix this by kfree'ing it and also > setting *pp_ent to NULL to be safe. > > Found with static analysis by CoverityScan,

RE: [PATCH] qed: fix memory leak of a qed_spq_entry on error failure paths

> From: Colin Ian King > > A qed_spq_entry entry is allocated by qed_sp_init_request but is not kfree'd > if an error occurs, causing a memory leak. Fix this by kfree'ing it and also > setting *pp_ent to NULL to be safe. > > Found with static analysis by CoverityScan, CIDs 1389468-1389470 > >

Re: linux-next: manual merge of the net tree with Linus' tree

> Today's linux-next merge of the net tree got a conflict in: >   drivers/net/ethernet/qlogic/Kconfig > between commit: >   2e0cbc4dd077 ("qedr: Add RoCE driver framework") > from Linus' tree and commit: >   0189efb8f4f8 ("qed*: Fix Kconfig dependencies with INFINIBAND_QEDR") > from the net

Re: linux-next: manual merge of the net tree with Linus' tree

> Today's linux-next merge of the net tree got a conflict in: >   drivers/net/ethernet/qlogic/Kconfig > between commit: >   2e0cbc4dd077 ("qedr: Add RoCE driver framework") > from Linus' tree and commit: >   0189efb8f4f8 ("qed*: Fix Kconfig dependencies with INFINIBAND_QEDR") > from the net

RE: [PATCH] qed: fix old-style function definition

> > The definition of qed_get_rdma_ops() is not a prototype unless we add > > 'void' here, as indicated by this W=1 warning: > > > > drivers/net/ethernet/qlogic/qed/qed_roce.c: In function ‘qed_get_rdma_ops’: > > drivers/net/ethernet/qlogic/qed/qed_roce.c:2950:28: error: old-style > > function

RE: [PATCH] qed: fix old-style function definition

> > The definition of qed_get_rdma_ops() is not a prototype unless we add > > 'void' here, as indicated by this W=1 warning: > > > > drivers/net/ethernet/qlogic/qed/qed_roce.c: In function ‘qed_get_rdma_ops’: > > drivers/net/ethernet/qlogic/qed/qed_roce.c:2950:28: error: old-style > > function

RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error

> config INFINIBAND_QEDR > - tristate "QLogic qede RoCE sources [debug]" > + bool "QLogic qede RoCE sources [debug]" Given that the qedr submission is going to turn this back into a tristate, are you certain this is a good thing [from compilation coverage perspective]? > -

RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error

> config INFINIBAND_QEDR > - tristate "QLogic qede RoCE sources [debug]" > + bool "QLogic qede RoCE sources [debug]" Given that the qedr submission is going to turn this back into a tristate, are you certain this is a good thing [from compilation coverage perspective]? > -

RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error

> > > > While I don't mind, you could have argued is that we're not > > > > removing enough, not too much. > > > > I.e., perhaps the rdma_msix_* fields should also have been > > > > ifdef-ed instead. [in which case this solution would not have > > > > worked] > > > > > > That would add even more

RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error

> > > > While I don't mind, you could have argued is that we're not > > > > removing enough, not too much. > > > > I.e., perhaps the rdma_msix_* fields should also have been > > > > ifdef-ed instead. [in which case this solution would not have > > > > worked] > > > > > > That would add even more

RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error

> > > - if (cond) > > > + if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) && cond) > > > qed_rdma_dpm_bar(p_hwfn, p_ptt); > > > > Why not simply fix the qed_roce.h empty implementation? > > Mainly for consistency: we have a couple of interfaces that are called from >

RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error

> > > - if (cond) > > > + if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) && cond) > > > qed_rdma_dpm_bar(p_hwfn, p_ptt); > > > > Why not simply fix the qed_roce.h empty implementation? > > Mainly for consistency: we have a couple of interfaces that are called from >