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.

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

allow SAI_ROUTE_ATTR_NEXT_HOP_ID point to SAI_NULL_OBJECT_ID by default #253

Merged
merged 2 commits into from
Oct 24, 2016

Conversation

lguohan
Copy link
Collaborator

@lguohan lguohan commented Oct 7, 2016

When SAI_ROUTE_ATTR_NEXT_HOP_ID = SAI_NULL_OBJECT_ID, packet is dropped.
This allows user to change packet action from DROP to FORWARD using set API.
Otherwise, the behavior transition from DROP to FORWARD is undefined.

When SAI_ROUTE_ATTR_NEXT_HOP_ID = SAI_NULL_OBJECT_ID, packet is dropped.
This allows user to change packet action from DROP to FORWARD using set API.
Otherwise, the behavior transition from DROP to FORWARD is undefined.
@lguohan
Copy link
Collaborator Author

lguohan commented Oct 7, 2016

@itaibaz , @AshokDaparthi , @atitjain , @tushar-ty , @krambn, please review.

@lguohan
Copy link
Collaborator Author

lguohan commented Oct 7, 2016

@kcudnik , this PR give following error.

Processing saiwred_8h.xml
invalid default value 'SAI_NULL_OBJECT_ID' on SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID (sai_object_id_t)
make: *** [saimetadata.c] Error 1
lgh@acs-trusty6:/data/sai/meta$

Can you help?

* The next hop id can be a generic next hop object, such as next hop, next
* hop group. Directly reachable routes are the IP subnets that are
* directly attached to the router. For such routes, fill the router
* interface id to which the subnet is attached. IP2ME route adds a local
* router IP address. For such routes, fill the CPU port
* (#SAI_SWITCH_ATTR_CPU_PORT).
*
* When it is SAI_NULL_OBJECT_ID, then packet will be dropped.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm the understanding that NULL_OBJECT_ID can be set when packet action is set to TRAP before and the packet action will take precedence in that case. In other words, the packet will be dropped when packet action is one of the following: FORWARD, COPY, LOG, TRANSIT as mentioned in the comments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the understanding. packet action will take precedence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or its just cancel forward copy. So for LOG and COPY we still get a copy.

@johnarnold
Copy link
Contributor

Jenkins, test this please

@lguohan lguohan merged commit 25c0020 into opencomputeproject:master Oct 24, 2016
stcheng pushed a commit to sonic-net/sonic-swss that referenced this pull request Nov 17, 2016
… forward" (#138)

This change breaks the latest test as the current SAI implementation doesn't support
the mid-state that the route with packet action DROP is set with next hop ID and packet
action FORWARD by two steps. The implementation also prevents removing the default route
and create a new route instead. After the SAI implementation is fixed, this change
will be re-introduced.

ref: opencomputeproject/SAI#253
andriymoroz-mlnx pushed a commit to andriymoroz-mlnx/sonic-swss that referenced this pull request Dec 6, 2016
… forward" (sonic-net#138)

This change breaks the latest test as the current SAI implementation doesn't support
the mid-state that the route with packet action DROP is set with next hop ID and packet
action FORWARD by two steps. The implementation also prevents removing the default route
and create a new route instead. After the SAI implementation is fixed, this change
will be re-introduced.

ref: opencomputeproject/SAI#253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants