open-traffic-generator / snappi

Open Traffic Generator SDK in Python and Go
MIT License
68 stars 7 forks source link

Automatic value generation of some required fields (MAC address, object names, etc.) #181

Open octpetre opened 1 year ago

octpetre commented 1 year ago

PR comments were made here (https://github.com/openconfig/featureprofiles/pull/291#discussion_r927111803) regarding the default address to this mandatory parameter while creating an OTG ethernet device. OTG set-config This is possible in ATE (IxNetwork) and most tests don't need to know what MAC addresses to use. Could this be addressed?

ashutshkumr commented 1 year ago

@octpetre kindly describe the issue here itself (and motivation behind it).

Is this a request to automatically assign MAC addresses to each ethernet device configured in OTG ? What's the priority for this ? Is it blocking any test case or just a quality of life improvement ?

Is it feasible to have a test helper which automatically provides unique MAC addresses ?

octpetre commented 1 year ago

For now the customer is fine with specifying the MAC address. However I think this is totally unnecessary, unless you're really interested in the actual address. Think about how many times you configured the MAC address in IxNetwork, so yes, it would be an improvement. A high level function as a helper could really help... See these comments below for a "GetorCreateDevice" high level function (https://github.com/openconfig/featureprofiles/pull/240#discussion_r905161206)

Cobular commented 1 year ago

I agree with octpetre that if no mac is provided, choosing a random one as the default would be a good implementation. However, if we are worried about this kind of default functionality being unexpected (since not much else in OTG seems to behave in this proposed way), there could just be a helper function that generates a mac and you just need to pass the value to the field. I ended up going with this approach in my project and wrote this helper, it's pretty trivial but would still be nice to have in the library.

from random import Random

rand = Random()

hex_digits = "0123456789abcde"

def seed_mac(seed):
    rand.seed(seed)

def make_mac() -> str:
    return "{}{}:{}{}:{}{}:{}{}:{}{}:{}{}".format(*rand.choices(hex_digits, k=12))
apratimmukherjee commented 1 year ago

Yes, i think it is a very valid requirement which we have discussed earlier as well. I think the auto-assignment can be done using very similar algorithm that IxNetwork uses . Keep first two bytes 00:00 ( prevents accidental matches with DUT MAC) Use next one to increment per port ( or two bytes as needed ) Last 1 or 2 bytes for intra port MAC increment. Instead of purely random values which makes it difficult to debug and allows more chance of conflict ( 2 MACs from Ixia should not ever be same ) IP should be mandatory though, no point in doing the auto assignment that IxN does, I think we almost always need to change it away. Keeping at it, another request I have had on similar lines is to be able to auto-assign unique name wherever we need a named element if user does not provide it. Mostly user would want to provide names only for elements he is interested in e,g. BGP Peer but probably does not really care to provide unique names for MAC / IP and VLAN layers. So why not auto-assign this which will make it so much easier to write the script.

ajbalogh commented 1 year ago

@apratimmukherjee I agree with allowing the mac fields to be auto but I think the auto assignment should be left to the system implementation as to what the mac addresses should be.

apratimmukherjee commented 1 year ago

Yes @ajbalogh . I agree but probably if we make MAC address non-mandatory , at least we should add the caveat that the implementation should ensure that auto-generated MAC addresses are unique across all interfaces configured in the test. And also possibly that care should be taken that auto-generated MACs should not collide with MAC addresses on connected DUTs .

For named fields , similar note should be mentioned esp. regarding expectation of uniqueness of a named field i.e e.g. globally or only for instances of that field etc. Note that this is also part of the comments provided in the PR in addition to comments regarding MAC ( https://github.com/openconfig/featureprofiles/pull/240#discussion_r904363374 )