-
Notifications
You must be signed in to change notification settings - Fork 17
RoCEv2 OTG Model #405
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
base: master
Are you sure you want to change the base?
RoCEv2 OTG Model #405
Conversation
config/config.yaml
Outdated
@@ -51,19 +51,26 @@ components: | |||
items: | |||
$ref: '../flow/flow.yaml#/components/schemas/Flow' | |||
x-field-uid: 6 | |||
stateful_flows: | |||
description: |- | |||
stateful flows currently on RoCEv2 present. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not for RoCEv2 alone. So probably something like "The stateful flows that will be configured for the traffic generator, for protocols such as RoCEv2 where packet rate and/or contents can change dynamically based on packets received from the connected device "
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@apratimmukherjee , @satyamsinghKey - I have doubt if we will change the packet content. For example for Roce, the content of the packet does not change. What changes dynamically/state-fully is packet rate (due to detection of congestion in the network), sequence of packet (for example the old packet may get send again due to dynamic retransmission) etc. In a prefect scenarios what changes in every packet is SEQ num. iCRC etc. But in the first reading it felt by "content" we are meaning payload, which does not change. So you may think of omitting the word "content".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead of 'contents' we can use 'contents of protocol headers' .. I mean flags in IPv4 header or window etc. for TCP etc. which is unlike stateless flows where nothing really changes pkt to pkt .
flow/rocev2_flow.yaml
Outdated
x-field-uid: 3 | ||
rate: | ||
description: |- | ||
Two valid choices are line_rate and burst_mode, if line_rate is selected, per_port_settings will be used to configure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more clarity would be good here.
e.g. when line_rate mode is chosen , it is required that all flows with a tx endpoint on such a port must choose line_rate mode [ unless mixed mode is alowed ???]
In this case, all the rdma flows will transmit data in interleaved manner based on qp.message size as fast as possible but not exceeding the line_rate value configured in port.rate .
If burst_mode is chosen , all flows with Tx endpoint on a port should be configured in burst_mode. Then each rdma flows sends packets as per rflow.message_size configuration for fixed / infinite times with burst_gap time between each iteration.
Some level of clarity on what is expected to go on the wire for each mode is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified description as suggested
Two valid choices are line_rate and burst_mode, if line_rate is selected, per_port_settings will be used to configure. | ||
type: string | ||
x-enum: | ||
line_rate: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
port_line_rate gives a hint to configure the line_rate in the port attributes .
type: integer | ||
format: uint64 | ||
x-field-uid: 5 | ||
connect_request_tx: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't these flap like regular tcp based sessions such as bgp ; any reason not to have a flap count.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can
rocev2_verb: | ||
description: >- | ||
RoCEv2 Verb, Available options are: none, write, wrtie_with_immediate, send, send_with_immediate and read: The corresponding flow will not take part in traffic. | ||
type: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a choice. Allows immediate_data to be made visible only for _immediate types. Looks like it is not relevant for the other types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct
format: hex | ||
default: "00000000" | ||
x-field-uid: 7 | ||
message_size: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is message_size part of session bring up or is it needed solely for data flows e.g. could a single QP potentially first send a payload of size 1000 bytes and then another of size 10000 bytes ? In which probably should be somewhere in stateful_flow configuration as properties of the data flow , maybe as an array with ixnetwork supporting a single value only for all flows between same qp endpoints. Or is this specification bound that each qp can have single datagram being split and sent on it ?
properties: | ||
connection_type: | ||
description: >- | ||
There are multiple connection types. Valid values are : Reliable Connection (RC), Reliable Datagram (RD), Extended Reliable Connection (XRC), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the time of this implementation, please consider only RC. So from description excludes other modes. Or, explicitly mention that supported mode is RC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not applicable for IxN but other test vendors whose configuration will be based on the model
x-field-uid: 7 | ||
message_size: | ||
description: >- | ||
The Maximum message size that is allowed to transfer depends on the MTU size and the number of VLANs configured on the interfaces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maximum bytes transferred will depend on IB MTU (not Ethernet MTU). So, given message size will be broken into multiple packets viz. WF, WM, WL etc. based on the IB MTU value. We take IB MTU as an user input also.
type: array | ||
items: | ||
$ref: '../flow/stateful_flow.yaml#/components/schemas/StatefulFlow' | ||
x-field-uid: 7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You cannot change x-uids of existing attributes. Please add new attributes to existing objects at the end else it will break clients running tests with older snappi versions.
Configuration for RoCEv2 peer settings. | ||
type: object | ||
properties: | ||
num_of_qps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flow settings is an array so number of times user add a new flow setting will define size of the flow settings array not this. Might need discussion.
type: object | ||
required: [num_of_qps, destination_ip_address, ib_mtu] | ||
properties: | ||
num_of_qps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to previous, the times user adds an element into the array will define the size .. should not be needing this .
description: >- | ||
Configuration for Rocev2 IPv4 peers. | ||
type: object | ||
required: [num_of_qps, destination_ip_address, ib_mtu] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name probably should be must , cant ib_mtu be non required with default value ?
num_qps probably can be removed completely.
description: >- | ||
Configuration for Rocev2 IPv6 peer settings. | ||
type: object | ||
required: [num_of_qps, destination_ip_address, ib_mtu] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
num_of_qps is probably just removable and ib_mtu should not be required since it has a default value.
In general every attribute should either have a default_value or be required but not both at the same time.
type: integer | ||
format: uint32 | ||
x-field-uid: 1 | ||
ib_mtu: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just move the ib_mtu to v4 and v6 peer and just remove this object. Model is easier to review and maintain. Or call it RoceAttributes and just have ib_mtu in that and refer to that . Dont need flow_settings which can be directly in the b4 / v6 peer.
x-field-uid: 1 | ||
tx_endpoint: | ||
description: |- | ||
Tx Endpoint. Empty for Rocev2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear what is meant by empty. Is it like RSVP-TE where the src defines the destination and must choose correct egress for specific ingress and no choice is available ?
It is not clear to me how this will help user control which QPs to actually send traffic on if this is empty. If a field is of no value we need to removed it.
on qp.message size as fast as possible but not exceeding the line_rate value configured. | ||
If burst_mode is chosen, all flows with Tx endpoint on a port should be configured in burst_mode. | ||
Then each rdma flows sends packets as per rocev2_flow_settings.message_size configuration | ||
for fixed / infinite times with burst_gap time between each iteration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just a suggestion. The text might need more clarity and accuracy in terms of what the different modes are trying to achieve which needs to come from deeper understanding of rocev2 traffic control needs,
If it is indeed required that certain modes require and mandate all flows on a port to exhibit same type of behaviour, current model would leave a lot of room for error in configuration . We probably need to rethink .
Maybe
rocev2_flows :
array of rocev2_ transmit ports :
port_name
choice
target_line_mode
line rate value desired across all qp values txed from this port
array of flows
src qp endpoint (only qps on this port can be selected)
optional dst port ( for tracking maybe if needed )
other flow parameters
burst_mode ( indivdual control per flow )
array of flows
tx endpoint
optional (expected ) rx endpoint for 2 arm flows and stats
burst_mode
choice
continuous
fixed
burst_count
Not clear whether inter_batch is for line_rate or burst_mode or both -> Needs to fitted in correctly accordingly with proper explanation and expectation from implementation what is the behaviour of that setting,
properties: | ||
inter_batch_period_unit: | ||
description: |- | ||
inter batch period units. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inter -> Inter
Needs more info on that all this means to packet generation by a tool implementing this,
inter_batch_period: | ||
description: |- | ||
Inter batch Period. | ||
type: integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
period.
A new PR has been created with the latest RoCEv2 model changes. Please refer to that PR for latest updates. |
Issue: #403
New PR Link: (#415)
Redocly View:
https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/open-traffic-generator/models/dev_rocev2/artifacts/openapi.yaml&nocors#tag/Configuration/operation/set_config
Objectives:
Add support of RoCEv2 Protocol in OTG model
Added fields under Configuration and Monitor
Sample Code Snippet: