Skip to content

Conversation

@sargun
Copy link
Contributor

@sargun sargun commented Dec 27, 2017

No description provided.

@aboch
Copy link
Collaborator

aboch commented Dec 30, 2017

Thanks.
Can you please also add the support where the filters are read from kernel:

func (h *Handle) FilterList(link Link, parent uint32) ([]Filter, error) {

After that, if possible, please add a test in filter_test.go to see whether you can retrieve a match_all filter after you programmed it.

@sargun
Copy link
Contributor Author

sargun commented Jan 2, 2018

@aboch It appears updated.


u32SelKeys := []TcU32Key{
TcU32Key{
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes were made by gofmt, I can split it out into another PR, or a different commit

}

func TestFilterClsActBpfAddDel(t *testing.T) {
tearDown := setUpNetlinkTest(t)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I no longer have a 3.10 kernel VM at hand to check this, but I am assuming this test would fail on older kernel.

Can you please add the call

minKernelRequired(t, x, y)

at the beginning of this test function, and move the same call at line 572 to the top of the new test function.

Thanks

@sargun
Copy link
Contributor Author

sargun commented Jan 5, 2018

@aboch updated.

tearDown := setUpNetlinkTest(t)
defer tearDown()
if err := LinkAdd(&Ifb{LinkAttrs{Name: "foo"}}); err != nil {
func setupLinkForTestWithQdisc(t *testing.T, linkName string) (Qdisc, Link) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@aboch
Copy link
Collaborator

aboch commented Jan 5, 2018

Thanks @sargun

@aboch aboch merged commit 1882fa9 into vishvananda:master Jan 5, 2018
@sargun sargun deleted the add-matchall branch January 5, 2018 17:44
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