DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wang, Xiao W" <xiao.w.wang@intel.com>
To: "luqiuwen@iie.ac.cn" <luqiuwen@iie.ac.cn>,
	"stable@dpdk.org" <stable@dpdk.org>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"yskoh@mellanox.com" <yskoh@mellanox.com>
Subject: Re: [dpdk-dev] [v2] net/fm10k: fix segment fault at calling fm10k_stats_get()
Date: Wed, 7 Aug 2019 02:20:41 +0000	[thread overview]
Message-ID: <B7F2E978279D1D49A3034B7786DACF407AF86D4D@SHSMSX106.ccr.corp.intel.com> (raw)
In-Reply-To: <b062a6a6-907a-4186-9951-08058ece5b69@DESKTOP-S1AP1MR.local>

Hi

> -----Original Message-----
> From: luqiuwen@iie.ac.cn [mailto:luqiuwen@iie.ac.cn]
> Sent: Tuesday, August 6, 2019 2:34 PM
> To: stable@dpdk.org; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W
> <xiao.w.wang@intel.com>; yskoh@mellanox.com
> Subject: [v2] net/fm10k: fix segment fault at calling fm10k_stats_get()
> 
> From 5658ee7d69023cc4b22315cd9f8e10a832aafdb0 Mon Sep 17 00:00:00
> 2001
> From: Lu Qiuwen <luqiuwen@iie.ac.cn>
> Date: Sat, 15 Jun 2019 14:28:01 +0800
> Subject: [PATCH] net/fm10k: fix segment fault at calling fm10k_stats_get()
>  from secondary process.
> 
> The function pointers in fm10k_stats_get() which setup from primary process,
> when secondary process call these function pointers, a segment fault will
> happend.

The patch tile is too long, we can simplify it to like "fix stats ops in multi process".
Also, the commit log exceeds the maximum length of 80 characters. Also some typos.
devtools/check-git-log.sh can be used to check your commit log format.

Since this is a fix patch, a "Fixes" line is needed. Refer to the other patches.
As a v2 patch, you need to specify what change you have made to the previous v1.

I'm OK with the below code change. After the above issue fixed, please add my ack:
Acked-by: Xiao Wang <xiao.w.wang@intel.com>

Maybe @Qi can help to reword the commit log when committing the patch?

BRs,
Xiao

> 
> Signed-off-by: Lu Qiuwen <luqiuwen@iie.ac.cn>
> ---
>  drivers/net/fm10k/base/fm10k_api.c | 20 ++++++++++++++++----
>  drivers/net/fm10k/base/fm10k_pf.c  |  4 ++--
>  drivers/net/fm10k/base/fm10k_pf.h  |  6 ++++++
>  drivers/net/fm10k/base/fm10k_vf.c  |  4 ++--
>  drivers/net/fm10k/base/fm10k_vf.h  |  5 +++++
>  5 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/fm10k/base/fm10k_api.c
> b/drivers/net/fm10k/base/fm10k_api.c
> index c49d20dfb..e7b2fe710 100644
> --- a/drivers/net/fm10k/base/fm10k_api.c
> +++ b/drivers/net/fm10k/base/fm10k_api.c
> @@ -234,8 +234,14 @@ s32 fm10k_read_mac_addr(struct fm10k_hw *hw)
>   * */
>  void fm10k_update_hw_stats(struct fm10k_hw *hw, struct fm10k_hw_stats
> *stats)
>  {
> -	if (hw->mac.ops.update_hw_stats)
> -		hw->mac.ops.update_hw_stats(hw, stats);
> +	switch (hw->mac.type) {
> +	case fm10k_mac_pf:
> +		return fm10k_update_hw_stats_pf(hw, stats);
> +	case fm10k_mac_vf:
> +		return fm10k_update_hw_stats_vf(hw, stats);
> +	default:
> +		break;
> +	}
>  }
> 
>  /**
> @@ -246,8 +252,14 @@ void fm10k_update_hw_stats(struct fm10k_hw *hw,
> struct fm10k_hw_stats *stats)
>   * */
>  void fm10k_rebind_hw_stats(struct fm10k_hw *hw, struct fm10k_hw_stats
> *stats)
>  {
> -	if (hw->mac.ops.rebind_hw_stats)
> -		hw->mac.ops.rebind_hw_stats(hw, stats);
> +	switch (hw->mac.type) {
> +	case fm10k_mac_pf:
> +		return fm10k_rebind_hw_stats_pf(hw, stats);
> +	case fm10k_mac_vf:
> +		return fm10k_rebind_hw_stats_vf(hw, stats);
> +	default:
> +		break;
> +	}
>  }
> 
>  /**
> diff --git a/drivers/net/fm10k/base/fm10k_pf.c
> b/drivers/net/fm10k/base/fm10k_pf.c
> index db5f4912f..f5b6a9e2e 100644
> --- a/drivers/net/fm10k/base/fm10k_pf.c
> +++ b/drivers/net/fm10k/base/fm10k_pf.c
> @@ -1511,7 +1511,7 @@ const struct fm10k_msg_data
> fm10k_iov_msg_data_pf[] = {
>   *  This function collects and aggregates global and per queue hardware
>   *  statistics.
>   **/
> -STATIC void fm10k_update_hw_stats_pf(struct fm10k_hw *hw,
> +void fm10k_update_hw_stats_pf(struct fm10k_hw *hw,
>  				     struct fm10k_hw_stats *stats)
>  {
>  	u32 timeout, ur, ca, um, xec, vlan_drop, loopback_drop,
> nodesc_drop;
> @@ -1584,7 +1584,7 @@ STATIC void fm10k_update_hw_stats_pf(struct
> fm10k_hw *hw,
>   *  This function resets the base for global and per queue hardware
>   *  statistics.
>   **/
> -STATIC void fm10k_rebind_hw_stats_pf(struct fm10k_hw *hw,
> +void fm10k_rebind_hw_stats_pf(struct fm10k_hw *hw,
>  				     struct fm10k_hw_stats *stats)
>  {
>  	DEBUGFUNC("fm10k_rebind_hw_stats_pf");
> diff --git a/drivers/net/fm10k/base/fm10k_pf.h
> b/drivers/net/fm10k/base/fm10k_pf.h
> index ca125c273..2c22bdd02 100644
> --- a/drivers/net/fm10k/base/fm10k_pf.h
> +++ b/drivers/net/fm10k/base/fm10k_pf.h
> @@ -184,4 +184,10 @@ extern const struct fm10k_msg_data
> fm10k_iov_msg_data_pf[];
>  #endif
> 
>  s32 fm10k_init_ops_pf(struct fm10k_hw *hw);
> +
> +void fm10k_update_hw_stats_pf(struct fm10k_hw *hw,
> +				     struct fm10k_hw_stats *stats);
> +
> +void fm10k_rebind_hw_stats_pf(struct fm10k_hw *hw,
> +				     struct fm10k_hw_stats *stats);
>  #endif /* _FM10K_PF_H */
> diff --git a/drivers/net/fm10k/base/fm10k_vf.c
> b/drivers/net/fm10k/base/fm10k_vf.c
> index bd449773a..2f4b5f5d2 100644
> --- a/drivers/net/fm10k/base/fm10k_vf.c
> +++ b/drivers/net/fm10k/base/fm10k_vf.c
> @@ -526,7 +526,7 @@ const struct fm10k_tlv_attr fm10k_1588_msg_attr[] =
> {
>   *
>   *  This function collects and aggregates per queue hardware statistics.
>   **/
> -STATIC void fm10k_update_hw_stats_vf(struct fm10k_hw *hw,
> +void fm10k_update_hw_stats_vf(struct fm10k_hw *hw,
>  				     struct fm10k_hw_stats *stats)
>  {
>  	DEBUGFUNC("fm10k_update_hw_stats_vf");
> @@ -541,7 +541,7 @@ STATIC void fm10k_update_hw_stats_vf(struct
> fm10k_hw *hw,
>   *
>   *  This function resets the base for queue hardware statistics.
>   **/
> -STATIC void fm10k_rebind_hw_stats_vf(struct fm10k_hw *hw,
> +void fm10k_rebind_hw_stats_vf(struct fm10k_hw *hw,
>  				     struct fm10k_hw_stats *stats)
>  {
>  	DEBUGFUNC("fm10k_rebind_hw_stats_vf");
> diff --git a/drivers/net/fm10k/base/fm10k_vf.h
> b/drivers/net/fm10k/base/fm10k_vf.h
> index 116c56fcc..d4edd330e 100644
> --- a/drivers/net/fm10k/base/fm10k_vf.h
> +++ b/drivers/net/fm10k/base/fm10k_vf.h
> @@ -89,4 +89,9 @@ extern const struct fm10k_tlv_attr
> fm10k_1588_msg_attr[];
>  	FM10K_MSG_HANDLER(FM10K_VF_MSG_ID_1588,
> fm10k_1588_msg_attr, func)
> 
>  s32 fm10k_init_ops_vf(struct fm10k_hw *hw);
> +
> +void fm10k_update_hw_stats_vf(struct fm10k_hw *hw,
> +				     struct fm10k_hw_stats *stats);
> +void fm10k_rebind_hw_stats_vf(struct fm10k_hw *hw,
> +				     struct fm10k_hw_stats *stats);
>  #endif /* _FM10K_VF_H */
> --
> 2.20.1.windows.1
> 


      reply	other threads:[~2019-08-07  2:20 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-06  6:34 Lu Qiuwen
2019-08-07  2:20 ` Wang, Xiao W [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B7F2E978279D1D49A3034B7786DACF407AF86D4D@SHSMSX106.ccr.corp.intel.com \
    --to=xiao.w.wang@intel.com \
    --cc=dev@dpdk.org \
    --cc=luqiuwen@iie.ac.cn \
    --cc=qi.z.zhang@intel.com \
    --cc=stable@dpdk.org \
    --cc=yskoh@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).