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

Add Shadow Services #140

Merged
merged 11 commits into from
Jun 20, 2019
Merged

Conversation

francescomessina
Copy link
Contributor

This PR adds the shadow mode on the cubes.
It adds span mode (only for shadow cubes).
It extends the pcn-router to manage netlink notifications
and take advantage of the shadow mode

@francescomessina francescomessina requested a review from a team as a code owner June 5, 2019 12:43
Copy link
Contributor

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

I added some general comments about the code.

Main worries are about span mode, I think we re wasting too mush resources for this support.
There is an additional perf ring buffer and thread for each cube, there are also additional lookups in the configuration map, I think it can be a good idea to try to implement this based on the conditional compilation.

src/polycubed/src/cube.cpp Outdated Show resolved Hide resolved
src/libs/polycube/include/polycube/services/cube.h Outdated Show resolved Hide resolved
src/polycubed/src/cube.cpp Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube_tc.h Show resolved Hide resolved
src/polycubed/src/cube.cpp Show resolved Hide resolved
src/polycubed/src/cube.cpp Outdated Show resolved Hide resolved
src/services/pcn-router/src/Ports.cpp Outdated Show resolved Hide resolved
src/services/pcn-router/src/UtilityMethods.cpp Outdated Show resolved Hide resolved
@francescomessina
Copy link
Contributor Author

francescomessina commented Jun 6, 2019

@mauriciovasquezbernal yes, I know, span mode consumes a lot of resources you're right, but I wouldn't know how to improve it, maybe the conditional compilation con help us.

src/libs/polycube/src/utils.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube.cpp Show resolved Hide resolved
src/polycubed/src/cube.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Outdated Show resolved Hide resolved
src/services/pcn-router/src/Router.cpp Outdated Show resolved Hide resolved
@frisso
Copy link
Contributor

frisso commented Jun 7, 2019

Francesco, still missing a little bit of documentation. We should integrate at least the concept of "shadow" service in the documentation: what it is, and what it does.
For the time being, I would avoid documenting explicitly the "span" flag. This should be mentioned, however, in a special section like "Debugging and tracing packets in Polycube services". In there, we can mention this new feature, which is very useful indeed.

@mauriciovasquezbernal
Copy link
Contributor

@mauriciovasquezbernal yes, I know, span mode consumes a lot of resources you're right, but I wouldn't know how to improve it, maybe the conditional compilation con help us.

Yes, please use conditional compilation. You can remove the map with the span flag and do a compilation time check with a #if, the logic should almost be the same. The only thing you need to add is that the cube is reloaded when the span mode changes.

@francescomessina francescomessina force-pushed the shadow_mode branch 2 times, most recently from b38b921 to cecc5ec Compare June 11, 2019 16:10
@acloudiator acloudiator added this to the Polycube Core milestone Jun 11, 2019
@francescomessina francescomessina force-pushed the shadow_mode branch 4 times, most recently from d91c131 to 61aa015 Compare June 17, 2019 15:03
- shadow = defines if the service is visible in Linux
- span = defines if all traffic is sent to Linux (valid only for shadow services)

Signed-off-by: francescomessina <francescomessina92@hotmail.com>
This commit adds a method to get the prefix from netmask,
and the one to get the netmask from the prefix.

Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Copy link
Contributor

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

I tested it, many of the features are working as expected:

  • namespace and ports are created when a service is in shadow mode
  • IPs in the polycube instance and in the namespace are updated accordingly
  • span mode allows to use wireshark, even in other services as bridge

There are some details to be considered:

  • there is not check for shadow when enabling span mode
  • I think the documentation has to be extended, why are these services neeed?, Me, as a user, when should use a shadow service?
  • is this possible to add automatic tests for some features?
    -- check if the namespace is created
    -- check if ip addresses are updated
    -- check if the routing table is updated
    -- something more?

src/polycubed/src/cube_tc.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube_tc.cpp Show resolved Hide resolved
src/polycubed/src/cube.cpp Outdated Show resolved Hide resolved
src/polycubed/src/cube.cpp Show resolved Hide resolved
src/services/pcn-router/test/test_shadow/test1.sh Outdated Show resolved Hide resolved
src/services/pcn-router/test/test_shadow/test2.sh Outdated Show resolved Hide resolved
Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Signed-off-by: francescomessina <francescomessina92@hotmail.com>
@francescomessina francescomessina force-pushed the shadow_mode branch 2 times, most recently from f769ac5 to 539f5e8 Compare June 18, 2019 13:24
This commit introduces the following points:
- create a namespace for each new shadow service
- create a veth and configure the its parameters (if available)
- creates a second "hidden" port for each new port created
  on the shadow service and connects it to the veth
- manages incoming traffic on the second port
  (traffic generated by the namespace) and forwards it out
- defines the method "pcn_pkt_redirect_ns()"
  useful to developers to send the packets to the namespace
- handles the Span mode directly in the cube_tc class (only if active)

Signed-off-by: francescomessina <francescomessina92@hotmail.com>
This commit register a Shadow router to the netlink events and
implements the functions to handle netlink notifications
  - Route added to the namespace
  - Route removed from the namespace
  - Interface removed
  - New Address on the interface
Local traffic and ARP traffic of a Shadow router is managed by the namespace.
The setIP, setNetmask and setMac methods have been implemented on the router.

Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Signed-off-by: francescomessina <francescomessina92@hotmail.com>
Copy link
Contributor

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks Francesco.

@frisso frisso merged commit 2692c3f into polycube-network:master Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants