open-traffic-generator / snappi Goto Github PK

FYI, I have not tested this in snappi, issue found with tgen, assuming implementation is the same.

If you have a traffic item with 16 flow groups and configure the tgen.flow to run until 10k packets are sent ( duration=Duration(FixedPackets(packets=packet_count) ) the traffic item is actually set to have 'Fixed Packet Count' 625 (10k/16 flow groups). This is fine for the scenario where the user wants a total of 10k packets sent no matter the flows/ports.

But we need to consider the scenario where the user actually wants to know how many packets it will be sending per port (which I believe will be more common). This user would expect that when setting packet_count=10k, he will see 10k per traffic item (and from that he could determine per port distribution). Right now this user would have to do the reverse math: if I want to send 10k per flow group, then I need to configure tgen to send 10K x flow groups...

So to overcome this, instead of having tgen silently doing the math, it should take the packet_count as is, and expose a setting for frame count distribution (and let the user decide how to split this count among flow groups).

In class DeviceList
the parameter "name" in ethernet, ipv4, ipv6 and bgpv4 is not accessed anywhere in the code.
setting the parameter while instance creation is not taking effect.

config.devices.ethernet(name="test") --> name is not taking effect and when serialized it gives None.

In this section: https://github.com/open-traffic-generator/athena/blob/dev-mon-ist/docs/hello-snappi.md#start-traffic
you show this snippet:

# start transmitting configured flows
ts = api.transmit_state()
ts.state = ts.START
api.set_transmit_state(ts)

This is a lot of lines of code to start the traffic. Is there a more compact way? For example:

api.set_transmit_state(api.transmit_state(ts.START))

This is still pedantic. How about something even easier, e.g.

api.transmit_state.start()

The same comment goes for stopping traffic, and starting capture.

These are my observation:

>> flowHeaderList = config.flows[0].packet
>> type(flowHeaderList)
<class 'snappi.flowheaderlist.FlowHeaderList'>
>> type(flowHeaderList[0])
<class 'snappi.flowethernet.FlowEthernet'>
>> type(flowHeaderList._items[0])
<class 'snappi.flowheader.FlowHeader'>

It looks like snappi is returning exact header (in this case 'FlowEthernet') rather than FlowHeader when iterate through FlowHeaderList though internal _items actually store FlowHeader.

IxNetwork concreate normally iterate through list object to check the choice of that object and take decision. say for this case

for flow_header in flowHeaderList :
        flow_header.choice

Please let me know if this is expected behavior then we will handle this accordingly.

Code:

import snappi
api = snappi.api()

d = api.details()
d.deserialize({'errors': []})

Traceback:

py/ethernet/test_basic_ethernet_snappi.py:17:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/usr/local/lib/python3.8/dist-packages/snappi/snappi.py:9579: in set_config
    return self._transport.send_recv(
/usr/local/lib/python3.8/dist-packages/snappi/snappicommon.py:41: in send_recv
    return return_object.deserialize(response_dict)
/usr/local/lib/python3.8/dist-packages/snappi/snappicommon.py:110: in deserialize
    self._decode(serialized_object)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _self = <snappi.snappi.Details object at 0x7f65e37fbb80>, obj = {'errors': [], 'warnings': []}    def _decode(self, obj):
        snappi_names = dir(self)
        for property_name, property_value in obj.items():
            if property_name in snappi_names:
                if isinstance(property_value, dict):
                    child = self._get_child_class(property_name)
                    if '_choice' in dir(child[1]) and '_parent' in dir(child[1]):
                        property_value = child[1](self, property_name)._decode(property_value)
                    else:
                        property_value = child[1]()._decode(property_value)
                elif isinstance(property_value,
>                               list) and property_name in self._TYPES:
E                               AttributeError: 'Details' object has no attribute '_TYPES'

 

The following invalid behavior needs to be fixed.

eth = flow.packet.ethernet()[-1]
eth.src.increment.start = '00:00:00:00:00:01'
eth.src.value # accessing the value getter switches the eth.src.choice from increment to value which is invalid (debugger watch will do this all the time)

parent and choice need to be passed to child choice classes which should only set the parent choice when a setter has been accessed.

the choice is not being set in the following

    eth = flow.packet[0]  # type: snappi.FlowEthernet
    assert (eth.__class__ == snappi.FlowEthernet)
    eth.src.value = '00:00:01:00:00:01'
    assert(eth.src.choice == 'value')

Most important usecase: fetching pcap binary from server

properties like port.name, flow.name etc which are mandatory to set.
those properties shall mandate to set when instance is created.
or
a validation mechanism needs to be added in set_config

Add coverage report with cov and make sure the coverage crosses 95%

Per https://github.com/open-traffic-generator/models/blob/master/port/port.yaml, name is a mandatory field and hence cannot be null. But following snippet works.

import snappi

api = snappi.api.Api()
config = api.config()
config.ports.port(location='localhost;2;1')
print(config.serialize())

Output:

{
  "ports": [
    {
      "location": "localhost;2;1",
      "name": null
    }
  ]
}

I do not find any straightway to validate if packet header fields are not setting at the time of configuration. I am using this https://github.com/open-traffic-generator/snappi/blob/main/snappi/tests/test_e2e_port_flow_config.py script to test it. ether_type is not configured within this script. These are my observation.

>>> packet
<snappi.flowethernet.FlowEthernet object at 0x000001338D7236A0>
>>> packet.ether_type
<snappi.flowpattern.FlowPattern object at 0x000001338DA42A90>
>>> packet.ether_type.choice
Traceback (most recent call last):
  File "C:\Program Files\JetBrains\PyCharm Community Edition 2017.3.2\helpers\pydev\_pydevd_bundle\pydevd_exec2.py", line 3, in Exec
    exec(exp, global_vars, local_vars)
  File "<input>", line 1, in <module>
  File "C:/MY_FOLDER/Sonic/Codebase/snappi\snappi\flowpattern.py", line 43, in choice
    return self._properties['choice']
KeyError: 'choice'
>>> hasattr(packet.ether_type, 'choice')
Traceback (most recent call last):
  File "C:\Program Files\JetBrains\PyCharm Community Edition 2017.3.2\helpers\pydev\_pydevd_bundle\pydevd_exec2.py", line 3, in Exec
    exec(exp, global_vars, local_vars)
  File "<input>", line 1, in <module>
  File "C:/MY_FOLDER/Sonic/Codebase/snappi\snappi\flowpattern.py", line 43, in choice
    return self._properties['choice']
KeyError: 'choice'

I found only way to validate it using this which is not user fraindly.

if 'choice' not in pattern._properties

I feel it will really helpful if we validate packet.ether_type is None

Currently we use host to specify api server address when instantiating api object.

api = snappi.api(host='https://localhost')

Although it has been raised that host might be an incorrect attribute name (since it only refers to ip address and TCP port pair).
In such cases, users might skip https:// schema. Following are the other options,

• addr
• uri / url
• location (consistent with port location)

What should we proceed with ?

@ajbalogh @ankur-sheth @winstonliu1111

If not supported, we'll need to add try/catch block around it.
https://docs.python.org/3/library/typing.html#typing.Literal

Examples I've seen create multiple flow objects like this, for example:

flow1, flow2 = self.cfg.flows.flow(name='f1').flow(name='f2')

However, it can be useful to allow one-at-a-time flow creation, especially for programmatic flow creation using loops where the number of flows cannot be hardcoded. For example,

flow2=snappi.Flow(name='f2')
self.cfg.flows.append(flow2)

If append() returned item you could do this which is much more compact and costs nothing.

flow2 = self.cfg.flows.append(snappi.Flow(name='f2'))

Flow.TxRx.Port

import snappi
api = snappi.api()
config = api.config()
f1, = config.flows.flow(name='f1')
f1.tx_rx.port.tx_name = 'p1'
print(config.serialize())

{
  "flows": [
    {
      "name": "f1",
      "tx_rx": {
        "port": {
          "tx_name": "p1",
          "rx_name": null
        },
        "choice": "port"
      }
    }
  ]
}

Flow.TxRx.Device

import snappi
api = snappi.api()
config = api.config()
f1, = config.flows.flow(name='f1')
f1.tx_rx.device.maps.map(tx_name='a', rx_name='b')
print(config.serialize())

{
  "flows": [
    {
      "name": "f1",
      "tx_rx": {
        "device": {
          "maps": [
            {
              "tx_name": "a",
              "rx_name": "b"
            }
          ]
        }
      }
    }
  ]
}

This is one code snippet for class FlowDevice(SnappiObject):

@property
def tx_names(self):
    # type: () -> list[str]
    """tx_names getter
    The unique names of devices that will be transmitting.
    Returns: list[str]
    """
    return self._get_property('tx_names')
@tx_names.setter
def tx_names(self, value):
    """tx_names setter
    The unique names of devices that will be transmitting.
    value: list[str]
    """
    self._set_property('tx_names', value)

Snappi is accepting string as well as return as string

>>> flow.tx_rx.device.tx_names = 'abcd'
>>> flow.tx_rx.device.tx_names
'abcd'

Our IxNetwork concreate we are looping according to specific attributes because OTG was taking care that part. I feel this is also little bit confused as description said " Returns: list[str]"

Please let me know your suggestion.

container_name=None, device_count=1
config.devices.device has the above device related arguments which shall be common for other wizard devices as well.
currently the above arguments can't be set for ethernet, ipv4, etc devices.

def device(self, container_name=None, device_count=1, name=None):
def ethernet(self, name=None):

An issue template should include at least following:

• Working python, API and OS version
• Failing python, API and OS version
• Testbed details (TBD)
• Example snippet or script that reproduces the issue
• Error stack or failure message
• Always reproducible or not

This needs more discussion. Will revisit after attaining snappi readiness.

defined error responses -> ResponseError
undefined error responses -> from proxy / ingress controller, e.g. nginx in k8s cluster

Autocomplete / intellisense works when using python REPL on Ubuntu but it doesn't seem to work on VSCode.

import snappi

api = snappi.api.Api()
config = api.config()
# got a suggestion for 'ports'
config.ports
# no suggestion for 'port()' or its kwargs
config.ports.port()

@ajbalogh I have temporarily reverted workflow changes in c229dcd since we need to create a new release.

We actually need to generate a new tag based on some input to be able to create release with latest changes.

1. The ports list is empty.
2. created new instance of layer1 and it returns list of 2 layer1 objects.
   -- it shall return only once instance not 2.

Usecase is to add a clone of first flow as third flow. Currently it's not feasible even if we use _add (private method).

This works:

import snappi
import copy

api = snappi.api.Api()
config = api.config()
f1, f2 = config.flows.flow(name='f1').flow(name='f2')

config.flows._add(copy.deepcopy(f1))
config.flows[2].name = 'f3'
print(config.serialize())

This does not work:

import snappi
import copy

api = snappi.api.Api()
config = api.config()
f1, f2 = config.flows.flow(name='f1').flow(name='f2')
f1.packet.ethernet().ipv4().tcp()
f2.packet.ethernet().ipv4().udp()

config.flows._add(copy.deepcopy(f1))
config.flows[2].name = 'f3'
print(config.serialize())

RecursionError: maximum recursion depth exceeded

• all logos relocated to https://github.com/open-traffic-generator/snappi-logo
• keep only colored logo png
• reduce dimensions to 500x100
• call it snappi-log.png and store it in both current and snappi-logo repo
• clean up current repo's history to reduce size from ~3.5MB to ~1.8MB

@ajbalogh @ankur-sheth @winstonliu1111 FYI

@ajbalogh
After creating the device object via config.devices.device(), right now the current implementation has access to create all the protocol containers.
if so, it would become problematic at concrete implementation.

device = config.devices.device()[0]
device.ethernet
device.ipv4
etc
please suggest is my understanding is correct.

It'd be nice to have a small library of utils to allow snappi and scapy to work together (if licensing issues are not an obstacle). For example, it'd be nice to be able to convert scapy packets to a flow packet spec and vice-versa. I found it useful to create packets in scapy to compare athena-captured packets against. It would be nice if I could have converted my transmit flow spec into scapy packets directly (e.g. I generated 255 packets with an increment pattern in the DIP, so give me those packets as a list of scapy packets which I can modify per my expected behavior in the DUT, then I can do a compare. FOr example of how I did it with scapy see https://github.com/chrispsommers/p4-guide/blob/c621ce0482f8c58b9bfddac545e997b473620f99/demo1-athena/ptf/demo1-snappi.py#L865.

Need to add the execution of pytest tests as part of the workflow action. This should happen after the wheel has been installed.

Snippet:

import snappi

api = snappi.api.Api()
config = api.config()
config.captures.capture()
print(config.serialize())

Error:

>       config.captures.capture()

self = <snappi.capturelist.CaptureList object at 0x7fc781faf850>, port_names = None, pcap = None, enable = True, overwrite = False
format = 'pcap', name = None

    def capture(self, port_names=None, pcap=None, enable=True, overwrite=False, format='pcap', name=None):
        # type: () -> Capture
        """Factory method to create an instance of the snappi.capture.Capture class
    
        Container for capture settings.
        """
>       item = Capture(port_names, pcap, enable, overwrite, format, name)
E       TypeError: __init__() takes from 1 to 6 positional arguments but 7 were given

snappi/capturelist.py:16: TypeError

device object in turn points to ethernet, ipv4, etc wizards. why device wizard is required when already protocol specific wizards does the work?

device = config.device()
device.ethernet
or
device.ipv4.ethernet
eth = config.ethernet()
eth.mac
ip = config.ipv4()
ip.ethernet

it feels like device wizard is extra over head and confuses the user

work flow:
config.devices.clear()
device=config.devices.device()[0]
device.ethernet.mac.value="00:00:00:00:00:01"

print(device)
container_name: null
device_count: 1
ethernet:
mac:
choice: value
value: 00:00:00:00:00:01
name: null
name: null

device choice field is not getting set even though mac address is set. and it can be seen only when name is set.

Snippet:

import snappi

api = snappi.api.Api()
config = api.config()
f = config.flows.flow(name='f1')
_, ip, _ = f.packet.ethernet().ipv4().tcp()
# IP address string
ip.src.value = '1.1.1.1'
# IP address int value
ip.dst.value = 64
print(config.serialize())

In models, the value for pattern should be string as specified in https://github.com/open-traffic-generator/models/blob/master/flow/packet-headers/patterns.yaml#L16.

Implication being, ipv4 dst addr value should've been "64" instead of 64.

Output:

{
  "flows": [
    {
      "name": "f1",
      "packet": [
        {
          "ethernet": {},
          "choice": "ethernet"
        },
        {
          "ipv4": {
            "src": {
              "metric_group": null,
              "choice": "value",
              "value": "1.1.1.1"
            },
            "dst": {
              "metric_group": null,
              "choice": "value",
              "value": 64
            }
          },
          "choice": "ipv4"
        },
        {
          "tcp": {},
          "choice": "tcp"
        }
      ]
    }
  ]
}

the methods used to get stats in https://github.com/open-traffic-generator/snappi/blob/main/docs/e2e_port_flow_config.py#L79 are no longer available on a snappi.Api object. Please update accordingly

import snappi
api = snappi.api()
config = api.config()

f, = config.flows.flow(name='f1')

# we want flow duration to use fixed packets
# 1st approach
f.duration.fixed_packets
# 2nd approach
f.duration.choice = f.duration.FIXED_PACKETS

print(config)

Output for 1st approach:

flows:
- duration:
    fixed_packets:
      delay: null
      delay_unit: null
      gap: null
      packets: null
  name: f1

Output for 2nd approach:

flows:
- duration:
    choice: fixed_packets
  name: f1

Proposed approach:

import snappi
api = snappi.api()
config = api.config()

f, = config.flows.flow(name='f1')

# we want flow duration to use fixed packets
# output 1
f.duration.fixed_packets.defaults()

# output 2
f.duration.fixed_packets
print(config)

# output 3
f.duration.choice = f.duration.FIXED_PACKETS
print(config)

Output:

# output 1
flows:
- duration:
    choice: fixed_packets
    fixed_packets:
      delay: null
      delay_unit: null
      gap: null
      packets: null
  name: f1

# output 2
flows:
- duration:
    choice: fixed_packets
  name: f1

# output 3
flows:
- duration:
    choice: fixed_packets
  name: f1

Snippet:

import snappi

api = snappi.api.Api()
config = api.config()
f = config.flows.flow(name='f1')
_, ip, _ = f.packet.ethernet().ipv4().tcp()
ip.src.value = '1.1.1.1'
# set invalid member
ip.dst.invalid_attr = 100
print(config.serialize())

The snippet above executes without issues, even though it should've raised validation error.

Output:

{
  "flows": [
    {
      "name": "f1",
      "packet": [
        {
          "ethernet": {},
          "choice": "ethernet"
        },
        {
          "ipv4": {
            "src": {
              "metric_group": null,
              "choice": "value",
              "value": "1.1.1.1"
            },
            "dst": {
              "metric_group": null
            }
          },
          "choice": "ipv4"
        },
        {
          "tcp": {},
          "choice": "tcp"
        }
      ]
    }
  ]
}

file url - /usr/local/lib/python3.8/dist-packages/snappi/snappicommon.py

In send_recv(self, method, relative_url, payload=None, return_object=None)

    `if response.ok:
        if response.headers['content-type'] == 'application/json':   
            return return_object.deserialize(yaml.safe_load(response.text))
        else:
            return None
    else:
        raise Exception(response.status_code, yaml.safe_load(response.text))`

print(response.headers)

{'Content-Type': 'application/json; charset=UTF-8', 'Date': 'Mon, 01 Feb 2021 17:04:14 GMT', 'Content-Length': '432'}

@ajbalogh wouldn't SnappiIter (base class) and ObjIter (derived class) be more appropriate ? We also have lesser chances of seeing Iter in node names.

Originally posted by @ashutshkumr in #49 (comment)

Allow pip install snappi[trex] to install snappi_trex==0.0.124

Using snappi with insecure localhost connaction results in annoying warnings:

import snappi
api = snappi.api(host='https://localhost:8080')
cfg = api.config()
res = api.set_config(cfg)
/usr/lib/python3/dist-packages/urllib3/connectionpool.py:999: InsecureRequestWarning: Unverified HTTPS request is being made to host 'localhost'. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/latest/advanced-usage.html#ssl-warnings
  warnings.warn(

I was advised to do this to suppress them:

urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)

How about an optional arg to snappi.api() such as allow_insecure=False which if set True will make this call under the hood. Also we might provide examples of using Athena/localhost with this option invoked.

• Put generator dependencies in requirements.txt or setup.py
• setup.py currently present in project dir should identify package snappi-gen and not snappi itself
• New setup.py should be generated (or duplicated from existing), which should be called snappi and include dependecies for generated code
• Generated code should be copied to be a separate directory (and hence snappicommon.py + other existing files should be copied to dir containing generated code)
• Generated code should also include .yaml or .json spec

#1 and #2 are subject to further discussion (if separate snappi-gen package is not desired).