Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <69d4df6f-71ea-4a64-b968-956c69812504@intel.com>
Date:   Tue, 21 Nov 2023 13:03:57 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v1] octeon_ep: get max rx packet length from
 firmware
Content-Language: en-US
To:     Shinas Rasheed <srasheed@marvell.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     <hgani@marvell.com>, <vimleshk@marvell.com>, <egallen@redhat.com>,
        <mschmidt@redhat.com>, <pabeni@redhat.com>, <horms@kernel.org>,
        <kuba@kernel.org>, <davem@davemloft.net>, <wizhao@redhat.com>,
        <konguyen@redhat.com>, Veerasenareddy Burru <vburru@marvell.com>,
        "Sathesh Edara" <sedara@marvell.com>,
        Eric Dumazet <edumazet@google.com>
References: <20231121191224.2489474-1-srasheed@marvell.com>
From:   Jesse Brandeburg <jesse.brandeburg@intel.com>
Autocrypt: addr=jesse.brandeburg@intel.com; keydata=
 xsFNBE6J+2cBEACty2+nfMyjkmi/BxhDinCezJoRM8PkvXlIGZL7SXAn7yxYNc28FvOvVpmx
 DbgPYDSLly/Rks4WNnVgAQA+nGxgg+tqk8DpPROUmkxQO7EL5TkszjBusUvL98crsMJVzoE2
 RNTJZh3ClK8k7r5dEePM1LM4Hq1bNTwE6pzyHJ1QuHodzR1ifDL7+3pYwt5wowZjQr4uJXFA
 5g5Xze8z0cnac+NpgIUqUdpEZ+3XmI92hIg2fUSRPUTgm+xEBijBv2OlTjZpzVfH8HlXeGCT
 E98Vuofvn2pgTZyJWJ6o0I9JUlxO+MMtMPuwL7Br0JqZQvvf80EFxbXnk+QSudg0sZAAec0g
 TSGWb7513siAqvAhxGjIf0cs2hEzRXbd4cVMZKPV2uai5g2LUsnS8m+zx/fzCC+KefKcxN8r
 Fs+9jNj2TOwmqahJqRBwxQZujNC96pkCQYzZtuz5BA7IMxC12TtnbvtUL6ef7GZVMv6b+rpe
 RmWnLIfGJItWefcse66l1wPQPi6tXmzBN6MaEDyVL6umiZTy7dnltaXsFZPPLapuk0qRoQtC
 aIjjk5VaK16t6pPUCRDW1um2anxOYBJCXzHrnzKf09hBgjbO2Tk5uKRQHpTEsm+38lIbSQ2r
 YUfOckMug/QHW05t+XVC2UuyAdjBamdvno7fhLaSTsqdEngqMQARAQABzTBKZXNzZSBDLiBC
 cmFuZGVidXJnIDxqZXNzZS5icmFuZGVidXJnQGludGVsLmNvbT7CwXgEEwECACIFAk6J+2cC
 GwMGCwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEKaiMWVzwKZycZ4QAIayWIWvnV2PiZ0E
 Kt7NMvSB3r3wx/X4TNmfTruURh24zrHcdrg6J8zSlXKt0fzxvvX7HYWgAEXD9BoVdPjh7TDy
 du9aMhFCFOfPHarz8DdGbT8UpGuX8bMZyd16/7nMqoGisK+OnmJubPxID2lDmXDRbxROahNF
 0ZJVXd+mw44FefzyJigJnfXtwyDuIit6ludKAs2iW3z298PuL13wiiG8rg5hTdWANxcC6wEh
 sycdt1JcKO6y5wcDwBr/yDPsUKaQPZTxRyiBK6NmQEN4BXbcG90VSgziJDPuYQb9ZOv2d0lX
 yidkXe/U9SpTSEcC6/Z8KinBl/5X/roENz5gW0H27m52Ht1Yx6SRpA3kwdpkzd0r5dKLCOVQ
 IwrAec5oLZRQqrSVp9+6PH7Z7YVQzN52nsgioQT8Ke2yht2ehsaJ97k718XhIWACyJqqmo/k
 wkj+5aUAi3ZXVOw3TGOpsfuz50Ods8CtGDHsUFwKlH10wXxOFdTa4PG+G4LTZ5ptkdFzm2rb
 9GJF2CSUS3ZMbBAQ/PZf1WpGUXBpOJMyD2AbWJQKTNn4yYMskMbnr4sGxitj6NHI4unlyd28
 1FmaRbR98v66sXYVVSP1ERFS/521OwMvWkPNuPMpqZ1ir9Nq/kw4t+urpVKF7RR87yuT46Gx
 /h2NVEXa750f7pf2LfPLzsFNBE6J+2cBEACfkrEDSsQkIlZzFgAN/7g0VmjHDrxxQSmvuPmZ
 L9pI6B/nNtclaUBu+q3rKUYBJhOfMobsafKOV8jYkENqOXvOvpb21t8HJ0FgqpMs+VE98gkp
 BM+Nitd+ePRJNScB8DKFmTT97QLBB8AdTWGy1tCSncoqhIz15X4ALplQkIoCuxdKPEuTeiyV
 mJFwvS0pB/GdN8hQEddRIo3E61dtLmSCH0iw6Zd8m9UHoZdZLWjfG+3EyeQ2TK0AFU9GpxVY
 nJ8mDacZlpcq4mjbr4w0G2IyjGyO6iLHKdYe3lU5Hs7lxZGbtnGQbGKL9VimV4IkKsXmTE+4
 /Mi+hWNxFBbZ7f7DUO3B7mZOicxxf2dK+vioHUr9TkWFwXARPwQGlGc3nGPQBhfaso+Q0q+b
 ftLhcdVDJjfNXvptWK3HbXQDsnkZ61nOEvjHDjpLQyzToKTSRoDNvnou2d26l5Nr7MHsqgxd
 xRKIau5xOAqO87AWHnbof3JW6eO8EDSmAYNWsmBBWFO7bfcJLyouiPSkDpsUniLh6ZAHyljd
 tYLPWatBqzvj28tTnA++Jp1bKDpby92GXQE2jZJ+5JCT+iW6dGQwrB9oMILx4V0WAvFsZT4t
 bq1MdS1n0qZD3t4ogYVqmYJyiB5ubTngI+s+VhDw3KbdhURJkQQ8dmojVfJZmeEH3u/eawAR
 AQABwsFfBBgBAgAJBQJOiftnAhsMAAoJEKaiMWVzwKZyTWQP/AlWAnsKIQgzP234ivevPc8d
 MOrOFslJrIutYqIW0V+B6teIcr73lejBl1fWtxn0mGPiTdNg/tJ48uN8K38yDzpxxmDDaKJa
 GGW6VPRezSpreqFjoEIz5NtJOo2dl7iK/6y7bAdlAeQj2Dvwj7Y1lB/JIbw8yoDg5Xl8D2db
 I8hchtsSXs8bxReEP1BGGsg4uyceOUexa1vAIGy80JDobbcjRaAo7xdwCXQjfEoC5UJVGd8g
 k21zDAUw3Eh47qO216txWwvOi+fq9o0UnOOAJ0xTRnQt1r5rMxEa8nLlChgfOSAdvBfaKAkn
 lIeWKK9LuETsiLpbofrey42d3wUUXggHYleYr9gR/7kQze78OATUHcud00B6EnmGDTOpbykp
 fby8AwgfbmcGz3LzgoZM7W9fnAkfVRuBOF5ge48kZecjHGxE69VB9180Aq6Bo2QVBlp3Le0j
 97DvMAwMgzyvfHHBPV0B9uzfxyBcxc9bRHXk0IiVIjm2e4gR+5WdsgXFd867ezQr3EiIe+6U
 +k7ZSjyrj7tsJOk1tKAvQKvMlxfRecw/yJDcKwwBHgEXVEnKgbu/Ci+ikbqsLCBWbOWs6eYq
 6m1nRM6nj0pgRDHIOQIxdWEysPWgmY2xxHb4yUq5YWa5+xu59zXdG72FqGqN8+Mkdw+M9m4D
 /fnLfll98Nhx
In-Reply-To: <20231121191224.2489474-1-srasheed@marvell.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?alI0Mk90Y0ltQU5zWTRGTXh1Nko2RUp1YWtJSVIyYlRKVTUxejdsQ2hKalZQ?=
 =?utf-8?B?QXU1VVJKRjkrd2t3SmoybE80Q05DNXJYMEFaK3N3R25McjNicU5DZTJRYW82?=
 =?utf-8?B?Q1JNV0tsVUhKN05tQkJIelRTNGYxQTBIUG10V0ZrdjBoTlNjTXJURGRNdlVz?=
 =?utf-8?B?ckZhdDE0Uk5Gak10U2NFcXhsUXhoRDZGMjBuT0tzOENFL3Izdi8yRDd1czB1?=
 =?utf-8?B?eEF4WUZhejV1MEJmSnZTWndNNjMzbjBWbHE0L3hDTTRtRXFPZlFnQTBubDZ3?=
 =?utf-8?B?OFpubVNhRGVxY3cybkNUYjlVRnhTY0R2dmdZYTd5YitaWFpQVUp4c3o2UXgz?=
 =?utf-8?B?WnlScThWdEphZkRyK0ZUVUlpTXpDRGVGdzlhbXdhZWJBM1lRNTdQN1BTY0pu?=
 =?utf-8?B?VjNOQ2pDN1p0cXBzVms1b0RHM0s2akc4QzA5ci8yc2oxY1h4WTRWaWhoZHdD?=
 =?utf-8?B?UStFNUp4UDhyQkJZWTRNNDhPdXBhTXdMK0FIaW13Q1FCcUlNL2gyMXFFdGJ3?=
 =?utf-8?B?UVNYcGZOaUF3RDNFZmRzbVNHOGp3dVZkckRZSlhFUks3b2hDU0xXN2xqeDJu?=
 =?utf-8?B?bXV6QnFaUjVNeGg5d0w2UGJiUjhxSWUxYktvL3MrcWwyUmNMT2R4K1RPREtY?=
 =?utf-8?B?NlpuNnk2S0g3T2JkOEw2dlJxbE9sNGRIbFFBd2l3RWpNSG1FVXlXaDlvL2lI?=
 =?utf-8?B?bGJQQmFQbkVHUjRvVXJRbFBwN0JKeTdBUHB5UUlzejNLbkg1MThqSUt4WEZD?=
 =?utf-8?B?dVVnVDY3czJjaXdoUURUMHZXY1NMQzlFUy8xZ2JYMEhxbGRsRElqMEFEdWQv?=
 =?utf-8?B?Vk02L2lueXJKVnVhR1RVNEd4aWhIRlZXekkvMnEyZ1VBMHBVdzlyT1JWaGdK?=
 =?utf-8?B?UTB0NjloQ1JHaU9OMnJEQWJnT2NiYi8wZ0t4aXVwQzFTMm5TaTJKbXBmYWcw?=
 =?utf-8?B?UzZMbjFGcXpxeDZUeGZ5TUlRcWFPSnBIK0o5UldpZEJlbzgwU04vTkRWNllp?=
 =?utf-8?B?c0tQUVhDWkZlMnhIenZmTFc2WktrSUFCRXlSclpoN1UzNW0vUHZYTzFLemcx?=
 =?utf-8?B?WDVOdXF5SXVvOVpiMWN0OGppTVYwajVYemEzZ3U1YnhhR0hCZ2dlR2FYRWhp?=
 =?utf-8?B?MmJTYUg2MUtxSk5SSzYrK0d5U0pxWVdyc0tyU016dTNWVEVKaWgwS2QzamtK?=
 =?utf-8?B?anlOS2t0Vm9yRGVJQXlqZE4vbk9uZDl1cE8xaEVDWEl6QVpvaVBXQkhyV2F4?=
 =?utf-8?B?ajZ1N0NkWjl2cmYvWlJkd3ZTVkJkQlk2cVpBWUNTL2EvMjB3NVNSNy92MFlX?=
 =?utf-8?B?K285dmFaNUlYaUpOano0SEZRSE1aRVI5ZElmZ2c4dFkwbE5PQ1BXTVNBSFZU?=
 =?utf-8?B?SkhxSjdDbzMyQzVRa2pnNG4xZCt0bmNyZ2YxNzRQazh5elpNNFovZkxNTFRo?=
 =?utf-8?B?NmtJNnpGa3pvbURiS0tSS2UrOU5XbDZ4SllkZkpabEI3eGp1Ky9rajZraGZu?=
 =?utf-8?B?QUpaNkFRKzNQYUZFK0Y0dDlHVTNqK1lCYlZlQ1NtNmlsM0hRSVpJV2d5MUpi?=
 =?utf-8?B?eS9BT28rQ0sveXkzKysxMG9SMHJsUnlQblppYzMvWkVWV0JCektNUXlZRGQ5?=
 =?utf-8?B?cUVxOW4wVExPZzBBTGQvamZrdE1jNFp0WS9zOHhxWmFmVWNIbWpITnVOS0Zk?=
 =?utf-8?B?TnFkRFpoc213eVM2QWx0TU9kYjIxdnp6RlB3OWk3SDV1dHhaakpJR2hIZ0k4?=
 =?utf-8?B?STM3QkFRdlNYZHlqNkxXUWpzdEVOVTFmMENQTE9sZWRCNWR3ZTYySE03YkIx?=
 =?utf-8?B?ZGwwR3h3aEdzTTlzM0VtSitwMzJHL3hWdy84ZEpDU3J1UzAxWDl2Qm1RUlR5?=
 =?utf-8?B?bDZkaW5FTEhad3FlV000OVh1Um9ORVYzWTRPajdTUHBCTkttYk40RTdKa0pp?=
 =?utf-8?B?eitqUU9xMVdQSzI1d05ZZGJWaFg3c3ZtZERkOVFLMHlrUStYZWJQaWJSTHRw?=
 =?utf-8?B?bUpxaXQrbUpIVFpTQUt1M3puT3l5TEloS2haNDg3UFdxRVJFSEtWVkpHYnFC?=
 =?utf-8?B?eTEyb1BORDI4cVBrNDRsbDlZaEs0SzRPL2xmYzVLTVN3SGhkUGxFZDlkTURm?=
 =?utf-8?B?bEZNdi9SU2xpYkRyOTZYZjRSNWpKbWorbUdDSllHR0J0M0RVZGRMSVNHdDYw?=
 =?utf-8?B?RWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9b5ca9c4-830a-4acf-41da-08dbead561b4
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4914.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Nov 2023 21:03:59.2289
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: un8nhb0EAi4VfMlHjwLKT3nZfn04kWZZn/qVgw0YEbn2gaqFuh6ypiZYrMS07PwodveXXsVpUBNR75ZkBBoaTpI1BRrmu0+FOT05dCm9W2A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4785
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Tue, 21 Nov 2023 13:04:44 -0800 (PST)

On 11/21/2023 11:12 AM, Shinas Rasheed wrote:
> Fill max rx packet length value from firmware.

Hi Shinas, thanks for the patch.

Please provide why, and make sure you're talking to the linux kernel
developer audience who don't know anything about your hardware. We're
interested to know why this patch is useful to the kernel and why it
might need to be applied.


> 
> Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
> ---
>  .../marvell/octeon_ep/octep_ctrl_net.c         | 18 ++++++++++++++++++
>  .../marvell/octeon_ep/octep_ctrl_net.h         |  9 +++++++++
>  .../ethernet/marvell/octeon_ep/octep_main.c    | 10 +++++++++-
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
> index 6dd3d03c1c0f..c9fcebb9bd9b 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
> @@ -198,6 +198,24 @@ int octep_ctrl_net_set_mac_addr(struct octep_device *oct, int vfid, u8 *addr,
>  	return octep_send_mbox_req(oct, &d, wait_for_response);
>  }
>  
> +int octep_ctrl_net_get_mtu(struct octep_device *oct, int vfid)
> +{
> +	struct octep_ctrl_net_wait_data d = {0};

I think preferred style is now d = { }; since that doesn't mess up if
the first member of the struct is an enum.

> +	struct octep_ctrl_net_h2f_req *req;
> +	int err;
> +
> +	req = &d.data.req;
> +	init_send_req(&d.msg, req, mtu_sz, vfid);
> +	req->hdr.s.cmd = OCTEP_CTRL_NET_H2F_CMD_MTU;
> +	req->mtu.cmd = OCTEP_CTRL_NET_CMD_GET;
> +
> +	err = octep_send_mbox_req(oct, &d, true);
> +	if (err < 0)
> +		return err;
> +
> +	return d.data.resp.mtu.val;
> +}
> +
>  int octep_ctrl_net_set_mtu(struct octep_device *oct, int vfid, int mtu,
>  			   bool wait_for_response)
>  {
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h
> index 4bb97ad1f1c6..46ddaa5c64d1 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h
> @@ -282,6 +282,15 @@ int octep_ctrl_net_get_mac_addr(struct octep_device *oct, int vfid, u8 *addr);
>  int octep_ctrl_net_set_mac_addr(struct octep_device *oct, int vfid, u8 *addr,
>  				bool wait_for_response);
>  
> +/** Get max MTU from firmware.
> + *
> + * @param oct: non-null pointer to struct octep_device.
> + * @param vfid: Index of virtual function.
> + *
> + * return value: mtu on success, -errno on failure.
> + */

The above block is definitely not correctly formatted kdoc (if that's
what you wanted), and you can probably get feedback about it from
scripts/kernel-doc -v
drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h

I see you have some correctly formatted doc in octep_tx.c


> +int octep_ctrl_net_get_mtu(struct octep_device *oct, int vfid);
> +
>  /** Set mtu in firmware.
>   *
>   * @param oct: non-null pointer to struct octep_device.
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> index 3cee69b3ac38..f9c539178114 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> @@ -1276,6 +1276,7 @@ static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	struct octep_device *octep_dev = NULL;
>  	struct net_device *netdev;
> +	int max_rx_pktlen;
>  	int err;
>  
>  	err = pci_enable_device(pdev);
> @@ -1346,8 +1347,15 @@ static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	netdev->hw_features = NETIF_F_SG;
>  	netdev->features |= netdev->hw_features;
> +
> +	max_rx_pktlen = octep_ctrl_net_get_mtu(octep_dev, OCTEP_CTRL_NET_INVALID_VFID);
> +	if (max_rx_pktlen < 0) {
> +		dev_err(&octep_dev->pdev->dev,
> +			"Failed to get max receive packet size; err = %d\n", max_rx_pktlen);
> +		goto register_dev_err;
> +	}
>  	netdev->min_mtu = OCTEP_MIN_MTU;
> -	netdev->max_mtu = OCTEP_MAX_MTU;
> +	netdev->max_mtu = max_rx_pktlen - (ETH_HLEN + ETH_FCS_LEN);
>  	netdev->mtu = OCTEP_DEFAULT_MTU;

Not part of this patch, but was there a point to setting the mtu here
without telling the netdev? most of the time it seems sufficient to just
set max and min since the kernel default is already 1500 (which your
internal define also duplicates)

Mostly the code seems fine.