nvme-print: fix traddr transformation for ave discovery log#3264
nvme-print: fix traddr transformation for ave discovery log#3264igaw wants to merge 1 commit intolinux-nvme:masterfrom
Conversation
inet_ntop wants the network address structure as input not converted string. Fixes: 1571543 ("nvme: add ave-discovery-log command") Signed-off-by: Daniel Wagner <wagi@kernel.org>
There was a problem hiding this comment.
Pull request overview
Fixes transport-address (traddr) rendering for AVE discovery log output by passing the actual binary address bytes to inet_ntop() rather than an address-family string.
Changes:
- Correct
inet_ntop()source argument in stdout AVE discovery log printing. - Correct
inet_ntop()source argument in JSON AVE discovery log printing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| nvme-print-stdout.c | Fixes AVE avetraddr formatting by feeding inet_ntop() the binary traddr field. |
| nvme-print-json.c | Fixes AVE avetraddr JSON formatting by feeding inet_ntop() the binary traddr field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (adrfam == NVMF_ADDR_FAMILY_IP6) { | ||
| af = AF_INET6; | ||
| size = INET6_ADDRSTRLEN; | ||
| } | ||
|
|
||
| if (inet_ntop(af, libnvmf_adrfam_str(adrfam), dst, size)) | ||
| if (inet_ntop(af, traddr, dst, size)) | ||
| printf("%s: %s\n", field, dst); |
There was a problem hiding this comment.
print_traddr() defaults to AF_INET for any adrfam other than NVMF_ADDR_FAMILY_IP6. After switching inet_ntop() to use the real traddr bytes, this will mis-decode non-IP address families (e.g., PCI/IB/FC/LOOP) as an IPv4 address. Consider explicitly handling only NVMF_ADDR_FAMILY_IP4/NVMF_ADDR_FAMILY_IP6 (and skipping or printing raw bytes for others) to avoid incorrect output.
| if (adrfam == NVMF_ADDR_FAMILY_IP6) { | ||
| af = AF_INET6; | ||
| size = INET6_ADDRSTRLEN; | ||
| } | ||
|
|
||
| if (inet_ntop(af, libnvmf_adrfam_str(adrfam), dst, size)) | ||
| if (inet_ntop(af, traddr, dst, size)) | ||
| obj_add_str(o, k, dst); |
There was a problem hiding this comment.
obj_add_traddr() treats any adrfam other than NVMF_ADDR_FAMILY_IP6 as IPv4 (AF_INET). With the corrected inet_ntop() input pointer, this can now emit incorrect IPv4 strings when aveadrfam is PCI/IB/FC/LOOP. Please gate inet_ntop() to IP4/IP6 families only (and omit/hex-encode for others) to avoid misleading JSON output.
|
Thanks for your fix. Just tested the changes with the dummy data as works correctly and the changes look good. Sorry for the error and previously I could not test the original changes correctly. |
|
Thanks! The copilot review has brought up some good points. I'll add them next week. running out of steam for today. |
inet_ntop wants the network address structure as input not converted string.
Fixes: 1571543 ("nvme: add ave-discovery-log command")
@ikegami-t did you tests the original implementation? If so, can you try this one too?