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

net: initial import of an IPv6 neighbor cache #2461

Merged
merged 2 commits into from
Mar 14, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 16, 2015

Depends on #2433 merged
Depends on #2546 for debug output merged

@miri64 miri64 added NSTF State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet State: waiting for other PR State: The PR requires another PR to be merged first Area: network Area: Networking Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Feb 16, 2015
@miri64 miri64 added this to the Network Stack Task Force milestone Feb 16, 2015
@@ -0,0 +1,40 @@
/*
* Copyright (C) 2014 Martin Lenders <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

Aren't this date & name deprecated? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It seems I have a broken template somewhere :D

Copy link
Member Author

Choose a reason for hiding this comment

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

(This file is part of #2433 but thanks, never the less)

@OlegHahm OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Feb 17, 2015
@miri64 miri64 mentioned this pull request Feb 19, 2015
36 tasks
@miri64
Copy link
Member Author

miri64 commented Feb 20, 2015

Rebased to current master

@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Feb 20, 2015
@miri64 miri64 mentioned this pull request Feb 20, 2015
/**
* @brief The size of the neighbor cache
*/
#define NG_IPV6_NC_SIZE (NG_NETIF_NUMOF * 8)
Copy link
Member

Choose a reason for hiding this comment

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

is 8 big enough? I would definitely put an #ifndef... around this, so that the application designer - who might know more about expected node degree - can overwrite this.

@OlegHahm
Copy link
Member

I guess it's too earlier for a review, right?

@miri64
Copy link
Member Author

miri64 commented Feb 20, 2015

I think so too ;-)

@miri64 miri64 force-pushed the ng_ipv6_nc/feat/initial branch 2 times, most recently from cdf108b to f10d6ea Compare March 3, 2015 20:55
@miri64
Copy link
Member Author

miri64 commented Mar 3, 2015

No longer WIP (except for tests)

@OlegHahm OlegHahm added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Mar 3, 2015
@miri64
Copy link
Member Author

miri64 commented Mar 4, 2015

No longer WIP

@miri64
Copy link
Member Author

miri64 commented Mar 6, 2015

Depends on #2546 for debug output

@miri64 miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 6, 2015
@miri64
Copy link
Member Author

miri64 commented Mar 11, 2015

Rebased to current #2546.

};

test_ipv6_nc_add__success();
ng_ipv6_nc_remove(TEST_UINT16 + 1, &addr);
Copy link
Member

Choose a reason for hiding this comment

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

Why + 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

To have another interface than the already registered one

@miri64
Copy link
Member Author

miri64 commented Mar 12, 2015

Addressed comments.

@OlegHahm OlegHahm removed the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 13, 2015
@OlegHahm
Copy link
Member

rebase is required.

TEST_ASSERT(ng_ipv6_addr_equal(&(entry->ipv6_addr), &addr));
TEST_ASSERT_EQUAL_STRING(TEST_STRING4, (char *)entry->l2_addr);
TEST_ASSERT_EQUAL_INT(sizeof(TEST_STRING4), entry->l2_addr_len);
TEST_ASSERT_EQUAL_INT(0, entry->flags);
Copy link
Member

Choose a reason for hiding this comment

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

If this the only additional test, why do we need a separate function?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is an interface global search (see https://github.com/RIOT-OS/RIOT/pull/2461/files#diff-bfdde70ccc3110439add7db00ff1edd6R200). But yes, this test is missing in test_ipv6_nc_get__success1();

@miri64
Copy link
Member Author

miri64 commented Mar 13, 2015

Rebased

@miri64
Copy link
Member Author

miri64 commented Mar 13, 2015

Addressed comments

@OlegHahm
Copy link
Member

ACK, please squash.

@miri64
Copy link
Member Author

miri64 commented Mar 13, 2015

Rebased

@miri64 miri64 removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 13, 2015
@miri64
Copy link
Member Author

miri64 commented Mar 13, 2015

Sigh, after 6 h of idling Travis realized there was some missing documentation. So here we go again -.-

miri64 added a commit that referenced this pull request Mar 14, 2015
net: initial import of an IPv6 neighbor cache
@miri64 miri64 merged commit 99e810e into RIOT-OS:master Mar 14, 2015
@miri64 miri64 deleted the ng_ipv6_nc/feat/initial branch March 14, 2015 01:26
@miri64
Copy link
Member Author

miri64 commented Mar 14, 2015

Finally :D

@OlegHahm
Copy link
Member

Woohoo!

@miri64
Copy link
Member Author

miri64 commented Mar 17, 2015

(now onwards to #2437?)

@OlegHahm
Copy link
Member

That's not my assignment. 😏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants