Skip to content

Conversation

krishd-amd
Copy link
Contributor

TESTING DONE:
On machine 1)
sudo shrub_controller -c 37 -d
In a new terminal
ZF_ATTR="interface=enp1s0f0;shrub_controller_id=37" ./build/gnu_x86_64/bin/zf_apps/static/zfsink 224.0.2.23:8080

With the above, i see the interfaces negotiate with the shrub_controller
ci Info: shrub_controller created a new server on interface with ifindex 3 buffer_count 4 hwport 5

zf_stackdump dump also shows shrub_controller_id

[krishd@dellr230t tcpdirect]$ sudo ./build/gnu_x86_64/bin/zf_stackdump dump
============================================================
onload version=547533a0d5 2025-09-08 master
tcpdirect version=9.1.0 23d32e4 2025-09-10 ON-15146
name=enp1s0f0/003 interface=enp1s0f0 vlan_id=65535
  pool: pkt_bufs_n=18048 free=18048
  config: tcp_timewait_ticks=666 tcp_finwait_ticks=666 ctpio_threshold=65535
  config: tcp_initial_cwnd=0 ms_per_tcp_tick=90
  alts: n_alts=0
  stats: ring_refill_nomem=0 cplane_alien_ifindex=0
         tcp_retransmits=0
  discards: discard_csum_bad=0 discard_mcast_mismatch=0
         discard_crc_bad=0 discard_trunc=0 discard_rights=0
         discard_ev_error=0 discard_other=0 discard_inner_csum_bad=0
         non_tcpudp=0
]
nic0: vi=3 vi_flags=3000000 nic_flags=1 intf=enp1s0f0 index=3 hw=5A0
  txq: pio_buf_size=0 added=0 removed=0
============================================================
UDP RX enp1s0f0/003:0
  filter: lcl=224.0.2.23:8080 rmt=0.0.0.0:0
  rx: unread=0 begin=0 process=0 end=0
  udp rx: release_n=0 q_drops=0
------------------------------------------------------------
---------------------attributes-------------------------------
tx_ring_max=512
rx_ring_max=512
tx_timestamping=0
rx_timestamping=0
ctpio=1
ctpio_mode=sf-np
rx_datapath=express
phys_address_mode=0
shrub_controller_id=37

If a user attempts to set the shrub_controller as -2, or 100000
[krishd@dellr230t tcpdirect]$ ZF_ATTR="interface=enp1s0f0;shrub_controller_id=10000" ./build/bin/zf_apps/static/zfsink 224.0.2.23:808
0
1000 | Bad shrub_controller_id; value must be -1 or in range 0 to 9999
ERROR: main: ZF_TRY(zf_stack_alloc(attr, &stack)) failed
ERROR: at src/tests/zf_apps//zfsink.c:265
ERROR: rc=-22 (Invalid argument) errno=0

If a user attempts to connect to a controller that is not running.
[krishd@dellr230t tcpdirect]$ ZF_ATTR="interface=enp1s0f0;shrub_controller_id=38" ./build/bin/zf_apps/static/zfsink 224.0.2.23:8080
1000 | Failed to allocate VI (rc = -2)
ERROR: main: ZF_TRY(zf_stack_alloc(attr, &stack)) failed
ERROR: at src/tests/zf_apps//zfsink.c:265
ERROR: rc=-2 (No such file or directory) errno=2
Aborted (core dumped)

@krishd-amd krishd-amd requested a review from a team as a code owner September 10, 2025 16:32
return -EINVAL;
}

rc = setenv("EF_SHRUB_CONTROLLER", shrub_controller_char , 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you prefer I adjust the ef_alloc_vi_from_pd to functions to take the shrub_controller parameter here? This just seems like an easier solution with fewer wider changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too sure how I feel about setenv here. It does indeed feel like it's probably the easiest way to get this done, but equally some kind of ef_vi_set_shrub_controller function might feel more pragmatic. I'm not too familiar with the structure of shrub though, so if such an approach is too cumbersome it might be a good idea to stick with setenv for now.

Copy link
Contributor

@jfeather-amd jfeather-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable, mainly just a few notes on avoiding magic numbers.

@@ -346,6 +346,13 @@ ZF_ATTR(int, phys_address_mode, stable, 0, NULL,
"\n"
"0 - Don't enable physical addressing mode. User space sees virtual addresses \n"
" which are translated by hardware or in the kernel.")

ZF_ATTR(int, shrub_controller_id, stable, -1, NULL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if appropriate to add here, but I think it would be nice to have a macro (or otherwise) for the "no shrub" value, e.g.:

#define ZF_SHRUB_CONTROLLER_ID_NO_SHRUB -1

return -EINVAL;
}

rc = setenv("EF_SHRUB_CONTROLLER", shrub_controller_char , 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too sure how I feel about setenv here. It does indeed feel like it's probably the easiest way to get this done, but equally some kind of ef_vi_set_shrub_controller function might feel more pragmatic. I'm not too familiar with the structure of shrub though, so if such an approach is too cumbersome it might be a good idea to stick with setenv for now.

@jfeather-amd jfeather-amd requested a review from a team September 12, 2025 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants