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 partial support for ipset #387

Merged
merged 2 commits into from
Sep 25, 2020
Merged

Conversation

corny
Copy link
Contributor

@corny corny commented Sep 27, 2018

Looking forward to read your comments and improve my code.

Copy link
Contributor

@jellonek jellonek left a comment

Choose a reason for hiding this comment

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

Great contribution!
Only minor documentation nits.

"golang.org/x/sys/unix"
)

type IPSetEntry struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Exported name without docstring.
Please add docstrings to all names which are started with upper case, using convention described in https://blog.golang.org/godoc-documenting-go-code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ipset_linux.go Outdated Show resolved Hide resolved
@corny corny force-pushed the ipset branch 4 times, most recently from f56c1f0 to bb40bd5 Compare September 30, 2018 23:35
@vishvananda
Copy link
Owner

This is looking good. Can you please squash it down into a single commit?

@corny corny force-pushed the ipset branch 3 times, most recently from f8761a6 to 0df1233 Compare October 31, 2018 12:33
@aboch
Copy link
Collaborator

aboch commented Nov 8, 2018

You may want to remove these two binaries from the diffset

image

@corny
Copy link
Contributor Author

corny commented Dec 8, 2018

You may want to remove these two binaries from the diffset

These are netlink responses. They are required by ipset_linux_test.go.

@corny corny force-pushed the ipset branch 2 times, most recently from bbde686 to f0450b3 Compare December 9, 2018 03:36
@aboch aboch mentioned this pull request Dec 11, 2018
@onokonem
Copy link

hi @corny

I have a problem with this patch

$uname -a
Darwin AF6438.local 18.2.0 Darwin Kernel Version 18.2.0: Fri Oct  5 19:41:49 PDT 2018; root:xnu-4903.221.2~2/RELEASE_X86_64 x86_64

$go version
go version go1.11.2 darwin/amd64

$GOOS=linux go build ./netmanager/cmd/netmanager
# github.com/AnchorFreePartner/services/vendor/github.com/vishvananda/netlink
../../vendor/github.com/vishvananda/netlink/ipset_linux.go:211:34: undefined: unix.NFNL_SUBSYS_IPSET

any idea why?

@corny corny force-pushed the ipset branch 2 times, most recently from 7b3d101 to e15c8f8 Compare April 28, 2020 17:18
@corny
Copy link
Contributor Author

corny commented Apr 28, 2020

@onokonem You don't have the proper header files on your system.

@corny
Copy link
Contributor Author

corny commented Sep 14, 2020

@vishvananda ping

@vishvananda
Copy link
Owner

@corny Thanks for updating this. Our policy is for prs to be a single commit. Can you squash this down? You can add a co-authored-by line for the original author so we don't lose their contributions.

@corny
Copy link
Contributor Author

corny commented Sep 17, 2020

I squashed it

@vishvananda vishvananda merged commit 1e3d26b into vishvananda:master Sep 25, 2020
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.

5 participants