#4964: Modify traffexam to be able to emit LACP data units#5060
Conversation
pkazlenka
commented
Feb 1, 2023
pkazlenka
commented
- Added new traffexam endpoint (/address//lacp)
pkazlenka
requested review from
artHalMirantis,
durgakako,
niksv,
rtretyak and
surabujin
* Added support to traffgen to emit LACP packet
* Added debug for endpoint
* Added new traffexam endpoint (/address/<idnr>/lacp)
* Fixed wrong explanation in comment
pkazlenka
force-pushed
the
tests/4964-add_lacp_support_to_traffexam
branch
from
4ac1932 to
5512ae4
Compare
* Bumped version (dev19 -> dev20)
|
Is it ok that the commit base is not the develop branch? Because in case of a merge, that code will not be in the develop. |
| try: | ||
| push_entry = model.LACPPush(payload) | ||
| get_context().action.lacp_push(address.iface, push_entry) | ||
| except ValueError as e: |
There was a problem hiding this comment.
What about KeyError(at least for missing keys in payload)?
There was a problem hiding this comment.
Added call to extract_payload_fields() method.
Yes. This change (as well as tests) doesn't make sense without changes from feature branch. I guess it's natural to have branches like |
* Updated LACP emitting code to be closer to other methods approach
| entry.aggregation, | ||
| entry.lacp_timeout, | ||
| entry.lacp_activity]: | ||
| actor_state_binary += self._bool_to_str(field) |
There was a problem hiding this comment.
why not actor_state_binary += '1' if field or '0'
| entry.lacp_timeout, | ||
| entry.lacp_activity]: | ||
| actor_state_binary += self._bool_to_str(field) | ||
| # Kilda ignores everything except lacp actor state. If this changes - expand this method with more fields |
There was a problem hiding this comment.
This is not true. Kilda stores info about partner's ports, system_id, keys, etc
docker/db-migration/migrations/024-add-lacp-partner-class.yaml
4 participants
