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

Support temporary view mode in syncd #133

Merged
merged 2 commits into from
Dec 12, 2016
Merged

Conversation

kcudnik
Copy link
Collaborator

@kcudnik kcudnik commented Dec 3, 2016

No description provided.

case SAI_OBJECT_TYPE_ROUTE:
case SAI_OBJECT_TYPE_TRAP:

SWSS_LOG_ERROR("get is not supported on %s in init view mode", sai_serialize_object_type(object_type).c_str());
Copy link
Contributor

@lguohan lguohan Dec 6, 2016

Choose a reason for hiding this comment

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

SWSS_LOG_ERROR("get is not supported on %s in init view mode", sai_serialize_object_type(object_type).c_str()); [](start = 16, length = 111)

why these objects are processed differently than the default? why cannot support for get for these types of objects?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because they dont exist, if you created route why do get on it ? You know what attributes you set on that route, so there is No sense to do get, similar for others


if (api != SAI_COMMON_API_GET)
{
// can't be called on init view, since some VID's may have no RID yet
Copy link
Contributor

Choose a reason for hiding this comment

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

// can't be called on init view, since some VID's may have no RID yet [](start = 8, length = 69)

we probably do not need this comment here as init view is handled completely differently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Leftover


SWSS_LOG_DEBUG("generic get (init view) for object type %s", sai_serialize_object_type(object_type).c_str());

// object must exists, we can't call GET on created object in init view mode
Copy link
Contributor

Choose a reason for hiding this comment

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

// object must exists, we can't call GET on created object in init view mode [](start = 20, length = 76)

when object does not exists, can you return the one in the database?

I am not sure what exactly are the use case here since we do not call get on the objects we created usually, but it would be good to add more comments here so that it might be easier to revisit this in the future.

Copy link
Collaborator Author

@kcudnik kcudnik Dec 6, 2016

Choose a reason for hiding this comment

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

You can call get on existing object like default trap Group or hash etc to get some vendor values

// all oid values here are VID's

snoop_get_response(object_type, str_object_id, attr_count, attr_list);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why is that the snoop op is only applied in the init view mode? The snoop op is trying to capture the asic state which are created by default in the sai library, so that we can use them for later comparison purpose.

for example, we first get 8 queues from asic, and remove 2 of them. In the asic view, we should have remaining 6. the snoop should be performed even during non init view phase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We will add that feature if it will be needed, so far is not needed , and queue example is more complex since after restart there muat be extra work performed to remove those queues, i mean on syncd restart, any remove will be dificult

@kcudnik kcudnik merged commit e393930 into sonic-net:master Dec 12, 2016
@kcudnik kcudnik deleted the temp branch December 12, 2016 04:11
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.

2 participants