netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hari Kalavakunta <kalavakunta.hari.prasad@gmail.com>
To: Paul Fertser <fercerpav@gmail.com>
Cc: Sam Mendoza-Jonas <sam@mendozajonas.com>,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, npeacock@meta.com,
	akozlov@meta.com
Subject: Re: [PATCH net-next 0/2] GCPS Spec Compliance Patch Set
Date: Tue, 8 Apr 2025 20:27:06 -0700	[thread overview]
Message-ID: <6d2abc68-ad40-4cfb-b6aa-bd724023f998@gmail.com> (raw)
In-Reply-To: <93ac7481-43c0-4207-8965-2d793c90263c@gmail.com>

On 4/8/2025 4:23 PM, Hari Kalavakunta wrote:
> On 4/8/2025 3:35 PM, Paul Fertser wrote:
>> On Tue, Apr 08, 2025 at 03:02:14PM -0700, Hari Kalavakunta wrote:
>>> On 4/8/2025 12:19 PM, Paul Fertser wrote:
>>>
>>>> In other words, you're testing your code only with simulated data so
>>>> there's no way to guarantee it's going to work on any real life
>>>> hardware (as we know hardware doesn't always exactly match the specs)?
>>>> That's unsettling. Please do mention it in the commit log, it's an
>>>> essential point. Better yet, consider going a bit off-centre after the
>>>> regular verification and do a control run on real hardware.
>>>>
>>>> After all, that's what the code is for so if it all possible it's
>>>> better to know if it does the actual job before merging (to avoid
>>>> noise from follow-up patches like yours which fix something that never
>>>> worked because it was never tested).
>>>
>>> I would like to request a week's time to integrate a real hardware
>>> interface, which will enable me to test and demonstrate end-to-end 
>>> results.
>>> This will also allow me to identify and address any additional issues 
>>> that
>>> may arise during the testing process. Thank you for the feedback.
>>
>> Thank you for doing the right thing! Looking forward to your updated
>> patch (please do not forget to consider __be64 for the fields).
> 
> I had not previously considered using __be64 for the struct 
> ncsi_rsp_gcps_pkt, as it is an interface structure. I would like to seek 
> your input on whether it is a good idea to use __be64 for interface 
> messages. In my experience, I haven't come across implementations that 
> utilize __be64. I am unsure about the portability of this approach, 
> particularly with regards to the Management Controller (MC).

Here are the results from a real hardware test, which validates the patch.


a. Initiate GCPS command to the NIC-2
root@bmc:~# ./ncsi-cmd -i 3 -p 0 -c 0 raw 0x18
<7> Command: type 0x18, payload 0 bytes:
<7> Send Command, CHANNEL : 0x0 , PACKAGE : 0x0, INTERFACE: 0x008cd598
<7> Response 228 bytes: 00 01 00 3b 98 00 00 cc 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 2b e5 fc e0 c0 00 00 00 00 
65 70 d5 54 00 00 00 00 09 50 66 56 00 00 00 00 03 08 a8 79 00 00 00 00 
00 3d 5c 44 00 00 00 00 00 79 38 23 00 00 00 00 00 02 fe 06 00 00 00 00 
00 00 02 be 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 b8 21 5f 03 8f da af 00 d8 6d 36 
00 73 a3 e7 00 17 20 70 00 00 00 00 00 00 d8 8a 00 00 0e 9c 00 56 4f 83 
00 10 17 e2 00 01 5b 76 00 13 6e a3 00 00 f8 cd 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 2c 20 55 f5

b. tcpdump capture of the GCPS
GCPS Command Capture:
         0x0000:  0001 003b 1800 0000 0000 0000 0000 0000
         0x0010:  ffff e7c4 0000 0000 0000 0000 0000 0000
         0x0020:  0000 0000 0000 0000 0000 0000 0000

GCPS Response
         0x0000:  0001 003b 9800 00cc 0000 0000 0000 0000
         0x0010:  0000 0000 0000 0000 0000 0000 0000 002b
         0x0020:  e5fc e0c0 0000 0000 6570 d554 0000 0000
         0x0030:  0950 6656 0000 0000 0308 a879 0000 0000
         0x0040:  003d 5c44 0000 0000 0079 3823 0000 0000
         0x0050:  0002 fe06 0000 0000 0000 02be 0000 0000
         0x0060:  0000 0000 0000 0000 0000 0000 0000 0000
         0x0070:  0000 0000 0000 0000 0000 0000 0000 0000
         0x0080:  0000 0000 0000 0000 0000 0000 0000 0000
         0x0090:  0000 0000 00b8 215f 038f daaf 00d8 6d36
         0x00a0:  0073 a3e7 0017 2070 0000 0000 0000 d88a
         0x00b0:  0000 0e9c 0056 4f83 0010 17e2 0001 5b76
         0x00c0:  0013 6ea3 0000 f8cd 0000 0000 0000 0000
         0x00d0:  0000 0000 0000 0000 0000 0000 0000 0000
         0x00e0:  2c20 55f5

c. dmesg log (Debug purpose only) to demonstrate correct value read.
[ 1350.369741] ncsi_rsp_handler_gcps ENTRY
[ 1350.369768] ncs->hnc_rx_bytes = 188542148800 (0x2be5fce0c0)


Let us examine the "Total Bytes Received" statistic. Specifically, we'll 
look at offset 28..35 (0x1c..0x24), bytes read - '0000 002b e5fc e0c0'. 
NCSI now correctly collects this value into it's internal structure.


I just noticed a warning regarding line length in the patch submission, 
I will address this issue in version 2 of the patch. Let me know if we 
need additional information.

  reply	other threads:[~2025-04-09  3:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-07 18:19 [PATCH net-next 0/2] GCPS Spec Compliance Patch Set kalavakunta.hari.prasad
2025-04-07 18:19 ` [PATCH net-next 1/2] net: ncsi: Format structure for longer names kalavakunta.hari.prasad
2025-04-07 18:19 ` [PATCH net-next 2/2] net: ncsi: Fix GCPS 64-bit member variables kalavakunta.hari.prasad
2025-04-07 22:23   ` Paul Fertser
2025-04-07 21:44 ` [PATCH net-next 0/2] GCPS Spec Compliance Patch Set Sam Mendoza-Jonas
2025-04-08 17:01   ` Hari Kalavakunta
2025-04-08 18:26     ` Paul Fertser
2025-04-08 18:53       ` Hari Kalavakunta
2025-04-08 19:19         ` Paul Fertser
2025-04-08 22:02           ` Hari Kalavakunta
2025-04-08 22:35             ` Paul Fertser
2025-04-08 23:23               ` Hari Kalavakunta
2025-04-09  3:27                 ` Hari Kalavakunta [this message]
2025-04-09  9:08                 ` Paul Fertser
2025-04-09 16:39                   ` Hari Kalavakunta

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=6d2abc68-ad40-4cfb-b6aa-bd724023f998@gmail.com \
    --to=kalavakunta.hari.prasad@gmail.com \
    --cc=akozlov@meta.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fercerpav@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=npeacock@meta.com \
    --cc=pabeni@redhat.com \
    --cc=sam@mendozajonas.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).