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

sys: net: add netdev device driver using dev_eth low-level ethernet driver API #2776

Merged
merged 2 commits into from
May 26, 2015

Conversation

kaspar030
Copy link
Contributor

Based on #2558. Depends on #2766. Obsoletes #2770.

This PR makes any ethernet device that implements #2766 available for the new network stack.

@kaspar030 kaspar030 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Apr 9, 2015
@kaspar030
Copy link
Contributor Author

(the test will not print any packets if pktdump's priority is not raised. see #2774)

@kaspar030
Copy link
Contributor Author

fixed travis unhappyness, rebased

@miri64 miri64 mentioned this pull request Apr 12, 2015
@jonas-rem
Copy link
Contributor

Hm, I am not sure if I can help here a lot. Have some trouble testing this.

  1. I checked out the ng_icmpv6/feat/initial branch (Like Martine described in the mailing-list)
  2. pulled ng_tapnet: initial import of a pure Ethernet device for native #2558 (ng_tapnet)
  3. pulled this one (sys: net: add netdev device driver using dev_eth low-level ethernet driver API #2776). Got some merge in this step
  4. set up tapnet using: cpu/native/tapsetup.sh create 2
  5. Building Riot was sucessful
  6. When starting Riot using: PORT=tap1 make term I got an error:
  51614     456   85920  137990   21b06 /home/jonas/git_repos/RIOT_MLENDERS/RIOT/tests/driver_netdev_eth/bin/native/driver_netdev_eth.elf
/home/jonas/git_repos/RIOT_MLENDERS/RIOT/tests/driver_netdev_eth/bin/native/driver_netdev_eth.elf tap0 
usage: /home/jonas/git_repos/RIOT_MLENDERS/RIOT/tests/driver_netdev_eth/bin/native/driver_netdev_eth.elf [-i <id>] [-d] [-e|-E] [-o]
 help: /home/jonas/git_repos/RIOT_MLENDERS/RIOT/tests/driver_netdev_eth/bin/native/driver_netdev_eth.elf -h

Options:
-h          help
-i <id>     specify instance id (set by config module)
-s <seed>   specify srandom(3) seed (/dev/random is used instead of
            random(3) if the option is omitted)
-d          daemonize
-e          redirect stderr to file
-E          do not redirect stderr (i.e. leave sterr unchanged despite
            daemon/socket io)
-o          redirect stdout to file (/tmp/riot.stdout.PID) when not attached
            to socket

The order of command line arguments matters.
make: *** [term] Error 1

Is this setup generally correct, what is missing there?

@kaspar030
Copy link
Contributor Author

Mostly.

  1. I take the master branch and then pull in @authmillenon's branch:
    # git pull https://github.com/authmillenon/RIOT.git ng_icmpv6/feat/initial
    Shouldn't make a difference.
  2. no need to pull ng_tapnet: initial import of a pure Ethernet device for native #2558, this PR includes everything
  3. actually there's no need for bridging. I precreate the tap using openvpn:
    # sudo openvpn --mktun --dev tap0
    Then setup the linux side of it (e.g., using ip to give it an address)
  4. I start the resulting binary directly:
    # <path>/bin/native/driver_netdev_eth.elf tap0

To test pinging,

  1. give the RIOT side an ipv6 address using, e.g., ifconfig <n> add 2001:db8::1/64
  2. add the linux side to the neighbour cache: ```ncache add 2001:db8::2
  3. do the opposite on the Linux side, make sure the tap is UP
  4. ping from Linux side should succeed

@kaspar030
Copy link
Contributor Author

I've added a patch that makes Linux/RIOT tap MAC addresses different.

@jonas-rem
Copy link
Contributor

Ok thx, hope I will find time to test this later.

@kaspar030 kaspar030 force-pushed the add_dev_eth_netdev2 branch from a7ccdc9 to 03cd524 Compare April 13, 2015 13:16
@kaspar030
Copy link
Contributor Author

rebased.

@jonas-rem
Copy link
Contributor

I can´t set the ipv6 addr using ifconfig. Is there an other dependency that enables setting the IP address using ifconfig via shell interface?

@miri64
Copy link
Member

miri64 commented Apr 14, 2015

@jremmert-phytec-iot yes, #2581. Sorry, I seem to have missed that in my original mail.

@jonas-rem
Copy link
Contributor

@authmillenon guess you meant #2582? But even with #2582 merged I am not able to set an ip address. I get the error: error: unable to add IPv6 address. Haven´t looked deeper into it because I don´t know how to debug native RIOT. I am not familiar with the native RIOT.

@OlegHahm My time to work on this and get used to it is limited. Maybe it would be more effective to assign someone else on this PR. Don´t want to delay the review process.

@OlegHahm
Copy link
Member

@jremmert-phytec-iot, as always there is no obligation to review a PR. An assignee can always decline and re-assign the PR. Feel free to chose another assignee if you don't have time to work on this.

@miri64
Copy link
Member

miri64 commented Apr 23, 2015

Needs rebase

@jonas-rem
Copy link
Contributor

@authmillenon, would you take over the assignment for this PR? Or is there someone else, that wants to take over the assignment? I haven´t the time to dig deeper into it, sry. Also I have no privileges for a re-assignment.

@miri64
Copy link
Member

miri64 commented Apr 23, 2015

I think I can do this.

return -EBADMSG;
}

hdr->type = byteorder_htons(ng_nettype_to_ethertype(pkt->next->type));
Copy link
Member

Choose a reason for hiding this comment

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

Nice to have: add NG_ETHERTYPE_RAW == 0xffff if hdr->type == 0. Otherwise Wireshark shows garbage. Alternatively you can fix ng_nettype_to_ethertype() if this is desired in any case.

@OlegHahm OlegHahm added this to the Release 2015.06 milestone Apr 29, 2015
@miri64
Copy link
Member

miri64 commented May 26, 2015

Please rebase.

@kaspar030 kaspar030 force-pushed the add_dev_eth_netdev2 branch from f9bbad0 to e85efd2 Compare May 26, 2015 17:35
@kaspar030 kaspar030 removed the State: waiting for other PR State: The PR requires another PR to be merged first label May 26, 2015
@kaspar030
Copy link
Contributor Author

Now that #2776 is merged:

  • rebased

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 26, 2015
@x3ro
Copy link
Contributor

x3ro commented May 26, 2015

I just tried this out, doing the following:

RIOT:

ifconfig 5 add 2001:db8::1/64

Linux:

brctl addbr br0
ip link set br0 up
sudo openvpn --mktun --dev tap0
ip link set tap0 up
brctl addif br0 tap0
sudo ip -6 address add 2001:db8::2/64 dev br0
ip -6 neigh add 2001:db8::1 lladdr <RIOT Mac> dev br0

Then I started the driver_netdev_eth test and ran ping6 2001:db8::1. The pings were seemingly received, but not responded to, by RIOT (this is from ng_netdev_eth):

ng_netdev_eth: Trigger ISR event
ng_netdev_eth: ISR event [ISR]
ng_netdev_eth: read 86 bytes
ng_netdev_eth: received packet from 06:84:28:54:a8:5e of length 72
000000 33 33 ff 00 00 01 06 84 28 54 a8 5e 86 dd 60 00
000010 00 00 00 20 3a ff 20 01 0d b8 00 00 00 00 00 00
000020 00 00 00 00 00 02 ff 02 00 00 00 00 00 00 00 00
000030 00 01 ff 00 00 01 87 00 46 f2 00 00 00 00 20 01
000040 0d b8 00 00 00 00 00 00 00 00 00 00 00 01 01 01
000050 06 84 28 54 a8 5e
ng_netdev_eth: get [not supported: 5]
ng_netdev_eth: get address

Where should I enable debug output?

@miri64
Copy link
Member

miri64 commented May 26, 2015

ip -6 neigh add 2001:db8::1 lladdr dev br0

What is this line for? It shouldn't be necessary anymore.

As far as I see you still do not configure the interface for forwarding:

sudo sysctl -w net.ipv6.conf.all.forwarding=1

I think this is not enough, but I'm not knowledgable enough in Linux router configuration to say this for sure. Maybe this (Gentoo-centric) link helps: https://wiki.gentoo.org/wiki/IPv6_router_guide

@x3ro
Copy link
Contributor

x3ro commented May 26, 2015

Just testing this PR for now. Wanted to see if the pinging works when I add it to the neighbor cache manually, and then test without adding it. And currently ping doesn't work for me :(

@miri64
Copy link
Member

miri64 commented May 26, 2015

But to the link-local address does, right?

@miri64
Copy link
Member

miri64 commented May 26, 2015

Sorry… I somehow thought I was commenting in #3049 ^^"

@miri64
Copy link
Member

miri64 commented May 26, 2015

But regardless… ping to link-local should work.

@kaspar030 kaspar030 force-pushed the add_dev_eth_netdev2 branch from e85efd2 to eee6de7 Compare May 26, 2015 19:07
@kaspar030
Copy link
Contributor Author

  • rebased, adapted to new THREAD_ defines

@miri64
Copy link
Member

miri64 commented May 26, 2015

Can you squash the eee6de7 into 8dda277, please?

@kaspar030 kaspar030 force-pushed the add_dev_eth_netdev2 branch from eee6de7 to 3a7820c Compare May 26, 2015 19:19
@miri64
Copy link
Member

miri64 commented May 26, 2015

👍

@miri64
Copy link
Member

miri64 commented May 26, 2015

ACK, this PR is well tested for my part.

@kaspar030
Copy link
Contributor Author

@authmillenon yup.

@x3ro I just tried again.

I added these to the tests Makefile:

USEMODULE += ng_ipv6_default
USEMODULE += ng_ipv6_ext
USEMODULE += ng_icmpv6_echo

Then compiled and started the test using

# sudo bin/native/driver_netdev_eth.elf tap0

Then added a ll address in riot:

> ifconfig 5 add fe80::1234/64

Then configured the linux-side:

# sudo ip link set up dev tap0

Then ping:

[kaspar@localhost riot.7]$ ping6 fe80::1234%tap0
PING fe80::1234%tap0(fe80::1234) 56 data bytes
64 bytes from fe80::1234: icmp_seq=1 ttl=64 time=1.01 ms
^C
--- fe80::1234%tap0 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 1.015/1.015/1.015/0.000 ms
[kaspar@localhost riot.7]$ 

@kaspar030
Copy link
Contributor Author

@authmillenon Should we just merge this? ;)

@kaspar030
Copy link
Contributor Author

perfect. Go!

kaspar030 added a commit that referenced this pull request May 26, 2015
sys: net: add netdev device driver using dev_eth low-level ethernet driver API
@kaspar030 kaspar030 merged commit acd9c14 into RIOT-OS:master May 26, 2015
@kaspar030 kaspar030 deleted the add_dev_eth_netdev2 branch May 26, 2015 19:25
@miri64
Copy link
Member

miri64 commented May 26, 2015

Ermmm… I would have been comfortable waiting for travis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants