-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Errors during SAI discovery #7895
Comments
yes, discovery don't respect conditions, just query all attributes for discovery, this should not be harmful, just a noise in syslog on error, we could enhance that to skip conditional attributes |
@stepanblyschak So after thinking a while, i see there is a problem here, since current discovery logic looks like this:
Problem here is, that for a given OID let say BRIDGE or BRIDGE_PORT, or any other type, we only have it's oid, and we don't have NON-OID attributes on which we could potentially deduce, whether next OID-ATTR query evaluate condition to true or false. In order to figure out whether we need to query that specific OID-ATTR (if conditional) we would need first query all it's condition attributes (non oids) and then determine whether condition is false, and don't query that OID attr, which will potentially prevent printing many error logs as you mentioned. If we for example have 1 conditional oid on BRIDGE_PORT, and it condition have 2 other attributes, we will need to query 2 attributes instead of 1 (if condition is false, and total 3 attributes if condition is true), and we would need to do that for each BRIDGE, since each bridge could have each condition in different state. Overall potentially over all switch we will query more attributes that there are oid attributes. This would cost us some time, and we want to do this operation as fast as possible. Also notice, that some of OID attributes, maybe not conditional at all, and they maybe just not implemented by that platform, and at this scenario there is no way to tell whether this attribute is supported on given object type, so the actual query is needed. And even if attribute is conditional, and condition is true, still current platform may not support that attr yet. Looking at SAI increasing more and more attributes, that number of attr will actually grow (non conditional and conditional oid attributes) when new version of SAI headers will be updated, so potentially there will be new and more errors each time that happens. Can't correctly interpret mlnx errors, whether that particular attributes eg. SAI_BRIDGE_ATTR_UNKNOWN_UNICAST_FLOOD_GROUP is not supported/not implemented since they point right before that "Unexpected bridge type 0 is not 1D". For not supported/implemented attributes, i think this should not be syslog level ERROR in my opinion rather WARNING, since that not implemented/not supported could be intentional behavior, or maybe they just left ERR just to notice. In your example, could you provide syslog, or a number, how many error logs is produced during single switch_create and discovery? And which attributes are reported, we could then check how many of them are conditional and which are not and see how we could prevent at least those conditional to be queried. |
These attributes cause errors. First one is Ok to check via condition but 2-4 are valid only for 1D bridge and it does not seem to be in metadata, just a comment in SAI - https://github.com/opencomputeproject/SAI/blob/master/inc/saibridge.h#L502 |
this valid for bridge type, is in comment, since SAI does not support mixed conditions yet (im working on it now, and this will be supported later) condition should be then like: @validonly bridge_attr_type == 1D and ( ... ) thanks for attaching syslog, form it i see that there are only those 4 attributes on BRIDGE which are producing errors, since there is only 1 bridge used in sonic (default one) and there are no other errors produced in manner of query OID attributes, i will keep this on my todo list, but with low priority since it's not critical, and i will fix this at some point |
@stepanblyschak I added support for SAI mixed conditions opencomputeproject/SAI#1255 so that validonly could be now fixed for bridge 1D here https://github.com/opencomputeproject/SAI/blob/master/inc/saibridge.h#L502 |
I corrected bridge 1D validonly condition, since mixed conditions support was added: opencomputeproject/SAI#1271 |
@kcudnik Is there a plan to fix this in syncd? |
yes, i already replied here: #7895 (comment) |
@kcudnik thanks! |
@kcudnik , @stepanblyschak where do we stands with this issue on master sonic? is it still open? will it be targeted to the next sonic/sai releases? |
i think this is still opened |
#10396) * Skip error messages for sonic-net/sonic-buildimage#7895
Summary: Skip err msg for the known issue : sonic-net/sonic-buildimage#7895 sonic-net/sonic-sairedis#582 This is a PR to fix the chery-pick conflict for the : #10396
Description
SAI discovery is tries to query all attributes on an object, even if some attributes are not valid for some cases.
E.g: querying SAI_BRIDGE_PORT_ATTR_PORT_ID on bridge port of type SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_1Q_ROUTER.
So, SAI discovery does not respect attr's "conditions".
Steps to reproduce the issue:
Describe the results you received:
Describe the results you expected:
Output of
show version
:Output of
show techsupport
:Additional information you deem important (e.g. issue happens only occasionally):
Cannot attach dump (some github issue), will retry later.

The text was updated successfully, but these errors were encountered: