Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixed conditions for ENCAP_MAPPERS and DECAP_MAPPERS #735

Merged
merged 2 commits into from
Dec 3, 2017
Merged

Fixed conditions for ENCAP_MAPPERS and DECAP_MAPPERS #735

merged 2 commits into from
Dec 3, 2017

Conversation

IGordynskyi
Copy link
Contributor

Fixed conditions for SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS tunnel attributes to attach to tunnel different types of mappers

Fixed conditions for SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATT…
b0b2c64 
…R_DECAP_MAPPERS tunnel attributes

to attach to tunnel different types of mappers

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>
@IGordynskyi
Copy link
Contributor Author

IGordynskyi commented Nov 16, 2017 •
edited
Loading

In current conditions for tunnel encap and decap mappers only SAI_TUNNEL_MAP_TYPE_OECN_TO_UECN and SAI_TUNNEL_MAP_TYPE_UECN_OECN_TO_OECN mappers can be attached to tunnel.
Removed condition section from tunnel encap and decap mappers to support attaching all types of mappers.
BTW: Are SAI_TUNNEL_ATTR_UNDERLAY_INTERFACE and SAI_TUNNEL_ATTR_OVERLAY_INTERFACE attributes mandatory on create only on IPINIP and IPINIP_GRE types? If not, need to change condition section in attribute description.

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 16, 2017

I approved only for code change, but FYI, if you remove those conditions, then those 2 attributes will be mandatory every time tunnel will be created

@itaibaz
Copy link
Collaborator

itaibaz commented Nov 16, 2017

i am not sure we want to make these attributes always mandatory. perhaps then we should remove the mandatory clause and give a default of empty list ? (but then we will miss the check that if user modes are defined, a map must be attached)

@vitaliy-senchyshyn
Copy link

vitaliy-senchyshyn commented Nov 16, 2017 •
edited
Loading

@itaibaz If we leave the code as is how the mappers of the types other than ECN related can be attached to the tunnel? We're trying to figure out how to connect bridge-to-vni mappers to the tunnel and looks like with the current definitions we can't do this.

Removed the mandatory clause and gave a default of empty list
bb79807 
in SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS definitions

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>
@IGordynskyi
Copy link
Contributor Author

@itaibaz I have pushed changes according to your comment. Could you please review?

@IGordynskyi
Copy link
Contributor Author

BTW, there are other inconsistencies in attributes definitions:

  1. Are SAI_TUNNEL_ATTR_UNDERLAY_INTERFACE and SAI_TUNNEL_ATTR_OVERLAY_INTERFACE attributes valid only on IPINIP and IPINIP_GRE types?
     * @type sai_object_id_t
     * @flags MANDATORY_ON_CREATE | CREATE_ONLY
     * @objects SAI_OBJECT_TYPE_ROUTER_INTERFACE
     * @condition SAI_TUNNEL_ATTR_TYPE == SAI_TUNNEL_TYPE_IPINIP or SAI_TUNNEL_ATTR_TYPE == SAI_TUNNEL_TYPE_IPINIP_GRE
  1. SAI_TUNNEL_ATTR_DECAP_TTL_MODE and SAI_TUNNEL_ATTR_DECAP_DSCP_MODE are valid only on IPINIP and IPINIP_GRE types and on encap TTL and DSCP modes don't have such conditions.

@karthik-krishnamurthy
Copy link
Contributor

@IGordynskyi Yes seems the attribute conditions have not been updated to accommodate the newer tunnel types and other changes which have happened in this file over time.

@IGordynskyi
Copy link
Contributor Author

@IGordynskyi Yes seems the attribute conditions have not been updated to accommodate the newer tunnel types and other changes which have happened in this file over time.

@karthik-krishnamurthy should that be fixed in scope of this PR?

@IGordynskyi
Copy link
Contributor Author

IGordynskyi commented Nov 28, 2017 •
edited
Loading

@kcudnik since this pull request is approved by you and @itaibaz could you please merge it?

@kcudnik kcudnik merged commit 451702e into opencomputeproject:master Dec 3, 2017
@kcudnik
Copy link
Collaborator

kcudnik commented Dec 3, 2017

Merged

@IGordynskyi
Copy link
Contributor Author

Thanks!

IGordynskyi pushed a commit to IGordynskyi/SAI that referenced this pull request Dec 5, 2017
Fixed conditions for ENCAP_MAPPERS and DECAP_MAPPERS (opencomputeproj…
1758ea6 
…ect#735)

* Fixed conditions for SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS tunnel attributes
to attach to tunnel different types of mappers

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>

* Removed the mandatory clause and gave a default of empty list
in SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS definitions

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>
lguohan pushed a commit that referenced this pull request Dec 12, 2017
@lguohan
Fixed conditions for ENCAP_MAPPERS and DECAP_MAPPERS (#735) (#743)
16ea53c 
* Fixed conditions for SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS tunnel attributes
to attach to tunnel different types of mappers

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>

* Removed the mandatory clause and gave a default of empty list
in SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS definitions

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>
kcudnik pushed a commit that referenced this pull request Mar 27, 2018
@manish-volansys @kcudnik
Added SAI_OBJECT_TYPE_BRIDGE_PORT support in ACL (#756)
e472cc2 
* Removing condition in bridge port bridge id attribute (#706) (#707)

* Fix condition in bridge port bridge id attribute

* Updating based on comments

* Fix indentation

* Remove whitespace

* Fix tagging mode flags/comments (#708) (#709)

* Fix tagging mode flags/comments

* Using validonly flag

* Add validonly and range tags to Doxygen (#717)

* merge CRM commits to v1.2 (#726)

* add SAI_SWITCH_ATTR_AVAILABLE_ACL_{TABLE,TABLE_GROUP} (#721)

SAI_SWITCH_ATTR_AVAILABLE_ACL_TABLE counts the available entires
for ACL TABLE objects

SAI_SWITCH_ATTR_AVAILABLE_ACL_TABLE_GROUP counts the available
entries for ACL TABLE GROUP objects

* add SAI_SWITCH_ATTR_AVAILABLE_NEXT_HOP_GROUP_ENTRY (#722)

* ACL CRM changes (#731) (#733)

* Critical Resource Monitoring related changed for ACLs

* Adapt to new gcc and doxygen (#725)

* Fixed conditions for ENCAP_MAPPERS and DECAP_MAPPERS (#735) (#743)

* Fixed conditions for SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS tunnel attributes
to attach to tunnel different types of mappers

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>

* Removed the mandatory clause and gave a default of empty list
in SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS definitions

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>

* Add return flag in notification data structure. (#745)

* Add return flag in notification data structure.

Add return flag in notification data structure to indicate app behavior on every deadlock situation.

* Update saiqueue.h

* Update saiqueue.h

* Update saiqueue.h

* Update saiserializetest.c

* Update saiserializetest.c

* Update saiserializetest.c

* Update saiqueue.h

* Update saivlan.h (#752)

Updated attribute comment

* Added SAI_OBJECT_TYPE_BRIDGE_PORT support in ACL

* Keep SAI_OBJECT_TYPE_BRIDGE_PORT along with SAI_OBJECT_TYPE_PORT in saiacl.h
karthik-krishnamurthy pushed a commit to karthik-krishnamurthy/SAI that referenced this pull request Apr 4, 2018
@karthik-krishnamurthy
Fixed conditions for ENCAP_MAPPERS and DECAP_MAPPERS (opencomputeproj…
3dae739 
…ect#735)

* Fixed conditions for SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS tunnel attributes
to attach to tunnel different types of mappers

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>

* Removed the mandatory clause and gave a default of empty list
in SAI_TUNNEL_ATTR_ENCAP_MAPPERS and SAI_TUNNEL_ATTR_DECAP_MAPPERS definitions

Signed-off-by: Iurii Gordynskyi <Iurii.Gordynskyi@cavium.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itaibaz itaibaz itaibaz approved these changes

@kcudnik kcudnik kcudnik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@IGordynskyi @kcudnik @itaibaz @vitaliy-senchyshyn @karthik-krishnamurthy