Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧹protobuf interface cleanup #397

Closed
baum opened this issue Jan 27, 2024 · 3 comments · Fixed by #402 or #417
Closed

🧹protobuf interface cleanup #397

baum opened this issue Jan 27, 2024 · 3 comments · Fixed by #402 or #417
Assignees

Comments

@baum
Copy link
Collaborator

baum commented Jan 27, 2024

gateway.proto suggested cleanups:

  1. enum TransportType could be removed. create/delete listener operation implementation should pass to SPDK transport type TCP
  2. enum AddressFamily should include two values: IPv4 and IPv6, preferably using SPDK styling, the rest of the options, IB and FC, should be removed.
  3. Probably there is no need to pass AutoHAState in create_listener_req, could behave is accordance with the subsystem HA configuration.
  4. message bdev_status is not referenced, should be removed.
  5. nit: trsvcid, representing TCP port, should be defined uint16 instead of uint32
@caroav
Copy link
Collaborator

caroav commented Jan 28, 2024

Agreed to do 1,2,4,5.
3 will be in another issue (less urgent at the moment).

@gbregman
Copy link
Contributor

@baum there is no type uint16 in protobuf.

gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 29, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 29, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 29, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
@gbregman
Copy link
Contributor

@baum as there is no 16 bit integers in protobuf I added a test in the CLI to give an error in case the port is 65536 or bigger.

gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 29, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 29, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 29, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 29, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 30, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 30, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 30, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 30, 2024
… and only allow IP addr family

Fixes ceph#397

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
baum pushed a commit to baum/ceph-nvmeof that referenced this issue Feb 2, 2024
Fixes: ceph#397, sub-issue 3
Remove `ana_reporting` & `auto_ha_state`. Use exclusively the subsystem enable HA flag

- pass to spdk as `ana_reporting` enable HA  subsystem flag
- listeners checks the subsystem enable HA flag

Signed-off-by: Alexander Indenbaum <aindenba@redhat.com>
baum pushed a commit to baum/ceph-nvmeof that referenced this issue Feb 2, 2024
Fixes: ceph#397, sub-issue 3
Remove `ana_reporting` & `auto_ha_state`. Use exclusively the subsystem enable HA flag

- pass the enable HA subsystem flag to spdk as ana_reporting
- listeners checks the subsystem enable HA flag

Signed-off-by: Alexander Indenbaum <aindenba@redhat.com>
baum pushed a commit that referenced this issue Feb 5, 2024
Fixes: #397, sub-issue 3
Remove `ana_reporting` & `auto_ha_state`. Use exclusively the subsystem enable HA flag

- pass the enable HA subsystem flag to spdk as ana_reporting
- listeners checks the subsystem enable HA flag

Signed-off-by: Alexander Indenbaum <aindenba@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
3 participants