xenanetworks / open-automation-chimera-core

XOA Chimera Core
https://docs.xenanetworks.com/projects/xoa-chimera-core
Apache License 2.0
0 stars 0 forks source link

MVP #1

Closed fpfeng closed 1 year ago

fpfeng commented 1 year ago

I'm currently working on MVP(minimum viable product) that we talk in meeting few days ago. Detail APIs was inside mvp.py, suggestions or ideas are welcome. I will update when MVP is finished.

fpfeng commented 1 year ago

after yesterday's meeting, here are some conclusion from it, order by priority.

fpfeng commented 1 year ago

@leonardhyu it seems like chimera module's M_TIMESYNC command is missing from driver's HLI dev branch. we still need to use it, right?

ArtemConstantinov commented 1 year ago

M_TIMESYNC is missing because it's marked as not supported by the Chimera module in BXMP Matrix. @fpfeng can you provide an example of where you want to use this command so for us, it will make easier to validate if there is any issue with commands matrix

fpfeng commented 1 year ago

1669967160547 this module setting as M_TIMESYNC

ArtemConstantinov commented 1 year ago

Yes, I can confirm through Vulcan Manager as the command is supported, @leonardhyu Please fix the matrix, after we are able to fix the driver image

leonardhyu commented 1 year ago

The API is called module.timing.source. Please check dev branch, and use the API map source file to check the LLI of each API.

leonardhyu commented 1 year ago

BXMP matrix updated to V8 and now it is available in FangCloud

fpfeng commented 1 year ago

The API is called module.timing.source. Please check dev branch, and use the API map source file to check the LLI of each API.

https://github.com/xenanetworks/open-automation-python-api/blob/dev/xoa_driver/internals/hli_v2/modules/module_chimera.py#L161

chimera module's ChTiming dont have .source

fpfeng commented 1 year ago

image @leonardhyu which ped_??? command should i use to get the current "enable" distribution, thanks. also can we access valkyrie manger source code so i can help myself looking in the code. 😊

leonardhyu commented 1 year ago

The command is called PED_GET, but we don't support it in XOA Driver because it is one of those commands that the response command is different from the request one. Similar to P_RATE ? where you get either P_RATEFRACTION or P_RATEL2BPS or P_RATEPPS back, when you send M/P PED_GET [flow, distribution_type] ?, the server will return the current distribution used by the flow of the port, as shown in the screenshot below. Because of this "weird" designed (for CLI it makes it easy for user to check the current setting), we have not yet included this type of commands in XOA Driver. So in your user code, you probably need to "remember" what distribution you have set for a flow. image

fpfeng commented 1 year ago

@leonardhyu Yes, I can remember which distribution that have been set, PED_GET is handy for a scenario the user just want to know current "set" distribution.

1670473128002

I notice if query get of "not being set" distribution then xenaserver return NOTVALID. I could check "get" result of each distribution to know if it have been set, it is workaround but maybe you guys would disagree this kind of behaviour. @ArtemConstantinov

ArtemConstantinov commented 1 year ago

Well, you can use a gather method with option of return exceptions, after to take only command which is not raised exception and that's will be your thing

fpfeng commented 1 year ago

@leonardhyu
I see there are two same "random" property of two different command. Maybe I messed up when refactor HLI2, should we rename to "random_rate" and "random_burst" or was there some reason using the same name? https://github.com/xenanetworks/open-automation-python-api/blob/a87a1634ac29d3a2c85b97775fe586736910d6d8/xoa_driver/internals/hli_v2/ports/port_l23/chimera/pe_distribution.py#L132-L145 image

leonardhyu commented 1 year ago

I'll check

fpfeng commented 1 year ago

image inside valkyrie manager, some impairment have different distribution, but same hli api. because using same distribution class?

        my_port.emulation.flow[0].misordering.depth
        my_port.emulation.flow[0].misordering.distribution.accumulate_and_burst
        my_port.emulation.flow[0].misordering.distribution.bit_error_rate
        my_port.emulation.flow[0].misordering.distribution.constant_delay
        my_port.emulation.flow[0].misordering.distribution.custom
        my_port.emulation.flow[0].misordering.distribution.fixed
        my_port.emulation.flow[0].misordering.distribution.fixed_burst
        my_port.emulation.flow[0].misordering.distribution.gamma
        my_port.emulation.flow[0].misordering.distribution.gaussian
        my_port.emulation.flow[0].misordering.distribution.ge
        my_port.emulation.flow[0].misordering.distribution.poison
        my_port.emulation.flow[0].misordering.distribution.random
        my_port.emulation.flow[0].misordering.distribution.uniform
        my_port.emulation.flow[0].misordering.distribution.step
        my_port.emulation.flow[0].misordering.distribution.off
        my_port.emulation.flow[0].misordering.enable
        my_port.emulation.flow[0].misordering.schedule
        my_port.emulation.flow[0].misordering.one_shot_status

(maybe it should post to drvier's repo)

ArtemConstantinov commented 1 year ago

@fpfeng something I don't like about aggregation of the filter config, looks as to nested access level for the user for be able to change an property, maybe we can provide an separate access for those subclasses, we also can use partialmethod for not expose enums

fpfeng commented 1 year ago

@fpfeng something I don't like about aggregation of the filter config, looks as to tasted access level for the user for be able to change an property, maybe we can provide an separate access for those subclasses, we also can use partialmethod for not expose enums

I was thinking about make it more nested level, LOL

   # current way
    current_filter_config.vlan.use_pcp_inner 
    current_filter_config.vlan.value_pcp_inner 
    current_filter_config.vlan.mask_pcp_inner

    current_filter_config.vlan.use_pcp_outer 
    current_filter_config.vlan.value_pcp_outer 
    current_filter_config.vlan.mask_pcp_outer 

   # future way
    current_filter_config.vlan.pcp_outer.use 
    current_filter_config.vlan.pcp_outer.value 
    current_filter_config.vlan.pcp_outer.mask

    current_filter_config.vlan.pcp_inner.use 
    current_filter_config.vlan.pcp_inner.value 
    current_filter_config.vlan.pcp_inner.mask

and then we privode helper method or proxy property for chang property, something like:

     current_filter_config.vlan.set_pcp_outer_use(True)
     current_filter_config.vlan.set_pcp_outer_value('0FFF')

@ArtemConstantinov @leonardhyu

ArtemConstantinov commented 1 year ago

I think this nesting of the filters need to be organized in different way, something like:

current_filter_config = await basic_filter_mode.get()

vlan = current_filter_config.use_l2_plus.use_vlan()
vlan.use.and()  # need to think about better naming
vlan.action_include()
vlan.use_tag_inner.on(
    value=20, 
    mask="0FFF"
)
vlan.use_pcp_inner.off()
vlan.use_tag_outer.off()
vlan.use_pcp_outer.off()

await basic_filter_mode.set(current_filter_config)
await flow.shadow_filter.make_active()

or

async with basic_filter_mode.get() as current_filter_config:
    vlan = current_filter_config.use_l2_plus.use_vlan()
    vlan.use.and()   # need to think about better naming
    vlan.action_include()
    vlan.use_tag_inner.on(
        value=20, 
        mask="0FFF"
    )
    vlan.use_pcp_inner.off()
    vlan.use_tag_outer.off()
    vlan.use_pcp_outer.off()

await flow.shadow_filter.make_active()

instead of:

current_filter_config = await basic_filter_mode.get()
logger.debug(current_filter_config)

current_filter_config.use_l2plus = enums.L2PlusPresent.VLAN1
current_filter_config.vlan.use = enums.FilterUse.AND
current_filter_config.vlan.action = enums.InfoAction.INCLUDE

current_filter_config.vlan.use_tag_inner = enums.OnOff.ON
current_filter_config.vlan.value_tag_inner = 20
current_filter_config.vlan.mask_tag_inner = "0x0FFF"

current_filter_config.vlan.use_pcp_inner = enums.OnOff.OFF
current_filter_config.vlan.value_pcp_inner = 0
current_filter_config.vlan.mask_pcp_inner = "0x07"

current_filter_config.vlan.use_tag_outer = enums.OnOff.OFF
current_filter_config.vlan.value_tag_inner = 20
current_filter_config.vlan.mask_tag_inner = "0x0FFF"

current_filter_config.vlan.use_pcp_inner = enums.OnOff.OFF
current_filter_config.vlan.value_pcp_inner = 0
current_filter_config.vlan.mask_pcp_inner = "0x07"

await basic_filter_mode.set(current_filter_config)
await flow.shadow_filter.enable(True)
fpfeng commented 1 year ago

I like first design, maybe it could be more simple:

# for use 2 vlan tag
vlan = current_filter_config.use_l2_plus.use_vlan(2)

vlan.tag_inner.on(...)
vlan.tag_outer.on(...)

vlan.tag_outer.off()
vlan.tag_inner.off()

# for use 1 vlan tag
vlan = current_filter_config.use_l2_plus.use_vlan(1)
vlan.tag.on(...)
vlan.tag.off()

current_filter_config.use_l2_plus.off()
ArtemConstantinov commented 1 year ago

Based on ValkiryManager you can use only one filter at the time for an layer, can be: use_none() or use_1_vlan_tag() or use_2_vlan_tags() or use_mpls().

Vlan Tag and Vlan Tags are a bit different tag can contain only value and mask but tags have explicit inner and outer specifications, simpler to create separate function with known names and avoid some magic numbers in function parameters

fpfeng commented 1 year ago

if just use 1 vlan tag it means "inner" only. https://docs.xenanetworks.com/projects/xoa-cli/en/latest/cli_ref/impairment/pef_commands.html?highlight=PEF_VLANTAG#pef-vlantag

so below api would be enough.

vlan.tag_outer.
vlan.tag_inner.
ArtemConstantinov commented 1 year ago

ok, just use_vlan(1) where 1 is an magic number probably won't work for us, instead will be better to return inner and outer tag and ask user to enable one or another or both during the configuration, but here is come up the question, which we need to clarify with @leonardhyu, What the reason on Valkyrie Manager we have separation of 1 tag or 2 tags when only 2 will be enough, and if it will be clear enough for user to not see vlan_1_tag