gnrc ipv6: append empty netif hdr for loopback by OlegHahm · Pull Request #5247 · RIOT-OS/RIOT

OlegHahm · 2016-04-04T23:47:26Z

The netif header gets discarded during the reverse procedure.

miri64 · 2016-04-05T09:33:31Z

What are the implications of this regarding the chosen interface?

OlegHahm · 2016-04-05T10:42:23Z

Not sure what you mean. The interface should be KERNEL_PID_UNDEF.

kaspar030 · 2016-04-05T11:13:17Z

I can confirm that this fixes the crash experienced in #4725.

miri64 · 2016-04-05T11:56:36Z

Not sure what you mean. The interface should be KERNEL_PID_UNDEF.

Yes, but how does IPv6 react to that fact in the long run?

kaspar030 · 2016-04-05T12:14:04Z

Yes, but how does IPv6 react to that fact in the long run?

Let's ask someone who must know... ;)

OlegHahm · 2016-04-05T12:37:09Z

I hope IPv6 stops sending RAs to the loopback interface in the long run. ;)

miri64 · 2016-04-05T13:27:16Z

Yes, but how does IPv6 react to that fact in the long run?

Let's ask someone who must know... ;)

The requirement for netif headers wasn't my idea, so I ask what the expected behavior for its iface value set to KERNEL_PID_UNDEF.

OlegHahm · 2016-04-05T13:34:01Z

I still don't quite understand the actual question.

miri64 · 2016-04-05T13:42:40Z

I want to know, if in the current implementation it is problematic when the given interface is set to KERNEL_PID_UNDEF and if no, why. Alternatively, if no, why is the netif header then required as assessed #5179. Wouldn't it then make more sense to revert #5179 and have iface = KERNEL_PID_UNDEF if netif == NULL.

miri64 · 2016-04-05T13:45:22Z

The more I think about it, I think reverting #5179 and amending the else-case is far less intrusive and restricting (given that KERNEL_PID_UNDEF doesn't lead to problems)...

OlegHahm · 2016-04-05T13:46:44Z

IMO KERNEL_PID_UNDEF is the correct interface identifier for loopback (as there is no loopback device in GNRC).

I still think that an upper layer should have the information about which interface a packet was received.

miri64 · 2016-04-05T13:48:53Z

Kk, but I repeat (and modify) my question from #5179: Can we think about other cases except loopback where KERNEL_PID_UNDEF might be a valid value?

OlegHahm · 2016-04-05T13:50:31Z

Yes.

miri64 · 2016-04-05T13:53:10Z

But then we can't assume KERNEL_PID_UNDEF to be the identifier for loopback, just that no interface information is available. So why require a netif header? The absence of it should be enough to implicitly assume KERNEL_PID_UNDEF (or equivalent values for addresses and RSSI/LQI).

OlegHahm · 2016-04-05T14:08:58Z

What about other link layer information? NDP or CCN requires information about the sender's link layer address, too.

miri64 · 2016-04-05T14:12:44Z

NDP doesn't require that. It should by (and is to my knowledge) able to also handle address-less link-layers as e.g. SLIP. As for CCN: how do you handle the loopback case then and isn't this applicable for other "interface-less" packets?

OlegHahm · 2016-04-05T14:24:49Z

There's no loopback in CCN.

miri64 · 2016-04-05T14:29:39Z

Then what has it to do with this PR / with the argument if a netif header is required for IPv6 or not?

OlegHahm · 2016-04-05T14:38:19Z

I don't know, you asked. Anyhow, I'm tired of discussing this over and over again. IMO it would be far more important to debug why RAs are sent over the loopback interface in the first place instead of discussing the same things again and again.

miri64 · 2016-04-05T14:47:43Z

I'm just advocating for questioning what we are doing before applying some "quick-fix" that opens a whole new well of problems, which then open another can and so on... Otherwise I fear we run into a similar situation as with the previous stack were nobody knows anymore what's going on exactly.

OlegHahm · 2016-04-08T11:15:00Z

For the release we need either to merge this or to revert #5179. I'm still convinced that this PR is the right solution, but if other people think differently or we cannot come to an agreement soon, I would be also fine with (temporarily) reverting #5179.

miri64 · 2016-04-08T11:37:47Z

Problem is, except the both of us nobody gave an argument to one or the other side. What is your opinion, @cgundogan @haukepetersen @kaspar030?

miri64 · 2016-08-05T13:06:56Z

How about defining -1 as system-wide as the identifier for loopback (also would be so beautifully symbolic because it mirrors [or in other terms negates] packets :D)

miri64 · 2016-08-05T13:08:15Z

(but we still shouldn't make netif-headers a requirement, just if the user wants to specifically address the loopback interface).

miri64 · 2016-08-30T18:21:17Z

ping?

OlegHahm · 2016-08-31T16:30:28Z

I haven't changed my mind.

miri64 · 2016-10-31T17:43:10Z

Postponed due to feature freeze

OlegHahm · 2017-01-14T14:11:55Z

I added a system-wide identifier for the loopback interface and rebased the PR. @miri64, what do you think?

miri64 · 2017-01-15T13:14:11Z


+/**
+ * @brief   Identifier of the loopback interface
+ */


I'd rather have it a positive number (maybe KERNEL_PID_MAX?), since I think there might be checks for >= KERNEL_PID_UNDEF in the code.

Actually, -1 was your proposal:
#5247 (comment)

KERNEL_PID_LAST + 1 would work for me, too.

Mh, then I trust past me for now. Let's test this :D

assertion fails at gnrc_ipv6.c:817 when pinging ::1 or one of the node's unicast link local addresses in the gnrc_networking application.

Considering that I chose -1 as NETIF_INVALID in #6413, I would actually prefer KERNEL_PID_LAST + 1 now.

aabadie · 2017-06-20T18:28:09Z

ping @OlegHahm @miri64, what should we do here ?

miri64 · 2017-06-20T21:11:42Z

Since a redo of GNRC's network interfaces is in the pipeline, I think I can include a loopback, but adding an empty netif header for loopback isn't the solution, due to its ambiguity.

kYc0o · 2017-06-30T13:58:41Z

So you propose to close this one?

miri64 · 2017-06-30T13:59:50Z

Nope, keep it open as a reminder. But I still don't agree that every packet must have a netif header on sending.

miri64 · 2017-11-15T18:13:14Z

I think I prefer this one to be closed now as well. I plan to add some proper loopback representation when #7925 is through anyways

OlegHahm assigned miri64 Apr 4, 2016

OlegHahm added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking GNRC labels Apr 4, 2016

OlegHahm added this to the Release 2016.04 milestone Apr 4, 2016

OlegHahm mentioned this pull request Apr 4, 2016

simplified border router setup #4725

Merged

OlegHahm mentioned this pull request Apr 9, 2016

gnrc ipv6: sending over loopback is broken #5279

Closed

miri64 mentioned this pull request Apr 11, 2016

gnrc_ipv6: add netif for loopback packets #5290

Closed

miri64 modified the milestones: Release 2017.01, Release 2016.10 Oct 31, 2016

OlegHahm added 3 commits January 14, 2017 15:11

gnrc ipv6: append empty netif hdr for loopback

e4ef748

gnrc: introduce loopback interface identifier

6b6db03

gnrc ipv6: set proper netif PID for loopback

b789680

OlegHahm force-pushed the gnrc_ipv6_loopback_netif_hdr branch from 3d35474 to b789680 Compare January 14, 2017 14:11

miri64 reviewed Jan 15, 2017

View reviewed changes

OlegHahm added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jan 18, 2017

PeterKietzmann added Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch and removed Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Jan 26, 2017

PeterKietzmann modified the milestones: Release 2017.04, Release 2017.01 Jan 31, 2017

kaspar030 modified the milestones: Release 2017.07, Release 2017.04 Apr 21, 2017

kYc0o modified the milestones: Release 2017.10, Release 2017.07 Jun 30, 2017

miri64 removed the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Nov 6, 2017

miri64 closed this Nov 15, 2017

miri64 added the State: archived State: The PR has been archived for possible future re-adaptation label Nov 15, 2017

Uh oh!

Conversation

OlegHahm commented Apr 4, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

kaspar030 commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

kaspar030 commented Apr 5, 2016 via email

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 5, 2016

Uh oh!

miri64 commented Apr 5, 2016

Uh oh!

OlegHahm commented Apr 8, 2016

Uh oh!

miri64 commented Apr 8, 2016

Uh oh!

miri64 commented Aug 5, 2016

Uh oh!

miri64 commented Aug 5, 2016

Uh oh!

miri64 commented Aug 30, 2016

Uh oh!

OlegHahm commented Aug 31, 2016

Uh oh!

miri64 commented Oct 31, 2016

Uh oh!

OlegHahm commented Jan 14, 2017

Uh oh!

miri64 Jan 15, 2017

Choose a reason for hiding this comment

Uh oh!

OlegHahm Jan 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 Jan 15, 2017

Choose a reason for hiding this comment

Uh oh!

miri64 Jan 15, 2017

Choose a reason for hiding this comment

Uh oh!

miri64 Jan 31, 2017

Choose a reason for hiding this comment

Uh oh!

miri64 Mar 16, 2017

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jun 20, 2017

OlegHahm Jan 15, 2017 •

edited

Loading