-
Notifications
You must be signed in to change notification settings - Fork 752
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 ConntrackDeleteFilters #989
Conversation
req2.AddRawData(dataRaw[4:]) | ||
req2.Execute(unix.NETLINK_NETFILTER, 0) | ||
matched++ | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this may use one comment to indicate the flow was already deleted so no need to process more filters and we can continue in the next flow
ConntrackDeleteFilters enables users to delete flow entries that match any of the specified filters. This allows users to delete multiple flow entries with a single dump table call. Signed-off-by: Daman Arora <aroradaman@gmail.com>
a0579cf
to
c96b03b
Compare
I was just looking for a function like this; hope it makes it in! |
cc. @aboch |
@@ -133,9 +139,9 @@ func (h *Handle) ConntrackUpdate(table ConntrackTableType, family InetFamily, fl | |||
return err | |||
} | |||
|
|||
// ConntrackDeleteFilter deletes entries on the specified table on the base of the filter using the netlink handle passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question for the maintainers, but they may prefer to leave the singular too for backwards compatibility purposes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is indeed a breaking change; the singular one above only took care of the function, but looks like it missed adding a Handle.ConntrackDeleteFilter
(singular) method on Handle
; see
73.74 # github.com/docker/docker/libnetwork/iptables
73.74 libnetwork/iptables/conntrack.go:78:28: nlh.ConntrackDeleteFilter undefined (type *netlink.Handle has no field or method ConntrackDeleteFilter)
73.74 libnetwork/iptables/conntrack.go:84:28: nlh.ConntrackDeleteFilter undefined (type *netlink.Handle has no field or method ConntrackDeleteFilter)
73.74 libnetwork/iptables/conntrack.go:105:13: nlh.ConntrackDeleteFilter undefined (type *netlink.Handle has no field or method ConntrackDeleteFilter)
@@ -311,7 +311,7 @@ func TestConntrackTableDelete(t *testing.T) { | |||
|
|||
// Flush entries of groupB | |||
var deleted uint | |||
if deleted, err = h.ConntrackDeleteFilter(ConntrackTable, unix.AF_INET, filter); err != nil { | |||
if deleted, err = h.ConntrackDeleteFilters(ConntrackTable, unix.AF_INET, filter); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may better add a new test or expand this to delete multiple flows with two different filters to exercise the new functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also considering passing the errors back to the caller. Currently, Execute()
can return an error, but it is not being captured.
Also returning the error will be difficult in case of multiple filters, we would have to decide if we want to exit immediately in case of error, or try out all the filters and then return the error at the end.
LGTM |
ConntrackDeleteFilters enables users to delete flow entries that match any of the specified filters. This allows users to delete multiple flow entries with a single dump table call.