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

Add support for REMOVE_BUCKET operation in group #45

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hongliangl
Copy link

Signed-off-by: Hongliang Liu lhongliang@vmware.com

@hongliangl hongliangl force-pushed the 20230113-support-remove-bucket branch from 44b5063 to 19fcf3c Compare January 17, 2023 01:27
Copy link

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

Please add test code for your new logic.

@@ -11,3 +11,4 @@

.idea/
.vscode/
vendor

Choose a reason for hiding this comment

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

Why adds this line?

Copy link
Author

Choose a reason for hiding this comment

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

I run command go mod vendor in this project by accident and found that this project doesn't ignore this directory.

Choose a reason for hiding this comment

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

But no directory named as "vendor" exists in this repo.

Copy link
Author

Choose a reason for hiding this comment

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

In Antrea, we also ignore this directory.

} else if command == openflow15.OFPGC_REMOVE_BUCKET {
// In a request of type OFPGC_REMOVE_BUCKET, the CommandBucketId field is used to specify the identifier of the
// bucket to remove from current action bucket list.
groupMod.CommandBucketId = self.Buckets[0].BucketId

Choose a reason for hiding this comment

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

Does it mean a single bucket can be removed in one OFPGC_REMOVE_BUCKET request? Is it possible to remove multiple buckets in a request?

Copy link
Author

Choose a reason for hiding this comment

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

From the document of Openflow 1.5, it cannot.

ofctrl/fgraphGroup.go Outdated Show resolved Hide resolved
ofctrl/fgraphGroup.go Show resolved Hide resolved
@wenyingd
Copy link

Please handle the golangci failure https://github.com/antrea-io/ofnet/actions/runs/3935221079/jobs/6730710061 , although it may not be introduced by your change.

@hongliangl hongliangl force-pushed the 20230113-support-remove-bucket branch 5 times, most recently from d777cbc to 49d9586 Compare January 17, 2023 03:37
This PR also upgrades golangci-lint version to v1.50.0 and
fix some fmt issues.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
@hongliangl hongliangl force-pushed the 20230113-support-remove-bucket branch from 49d9586 to e8f71cb Compare January 17, 2023 03:41
@hongliangl
Copy link
Author

Please handle the golangci failure https://github.com/antrea-io/ofnet/actions/runs/3935221079/jobs/6730710061 , although it may not be introduced by your change.

Please handle the golangci failure https://github.com/antrea-io/ofnet/actions/runs/3935221079/jobs/6730710061 , although it may not be introduced by your change.

Done

Add parameter `NewGroup` to decide whether to use cache in
OFSwitch when create a group.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
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