-
-
Notifications
You must be signed in to change notification settings - Fork 208
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
sFlow for tunnels: Tunnel extension & IPv6 encapsulation support #574
base: master
Are you sure you want to change the base?
Conversation
Thanks Simon. I will review the patch. |
I would propose to merge to two options "gre" and "6in4" into a more generic option "tunnel". That would offer more flexibility for future tunnel decoding '4in6' or whatever would be proposed. The single option should not cause conflicts in my view. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Propose one single option parse_tunnel
to include all current and future tunnel decodings.
@@ -96,7 +96,7 @@ static int verbose = 0; | |||
typedef ssize_t (*packet_function_t)(int, void *, size_t, int, struct sockaddr *, socklen_t *); | |||
|
|||
static option_t sfcapdConfig[] = { | |||
{.name = "gre", .valBool = 0, .flags = OPTDEFAULT}, {.name = "maxworkers", .valUint64 = 2, .flags = OPTDEFAULT}, {.name = NULL}}; | |||
{.name = "gre", .valBool = 0, .flags = OPTDEFAULT}, {.name = "6in4", .valBool = 0, .flags = OPTDEFAULT}, {.name = "maxworkers", .valUint64 = 2, .flags = OPTDEFAULT}, {.name = NULL}}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one option .name = "tunnel" ..
@@ -115,7 +115,7 @@ static void IntHandler(int signal); | |||
static inline FlowSource_t *GetFlowSource(struct sockaddr_storage *ss); | |||
|
|||
static void run(packet_function_t receive_packet, int socket, int pfd, int rfd, time_t twin, time_t t_begin, char *time_extension, int compress, | |||
int parse_gre); | |||
int parse_gre, int parse_6in4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One argument -> int parse_tunnel
@@ -272,7 +272,7 @@ static int SendRepeaterMessage(int fd, void *in_buff, size_t cnt, struct sockadd | |||
} // End of SendRepeaterMessage | |||
|
|||
static void run(packet_function_t receive_packet, int socket, int pfd, int rfd, time_t twin, time_t t_begin, char *time_extension, int compress, | |||
int parse_gre) { | |||
int parse_gre, int parse_6in4) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one option -> int parse_tunnel
int parse_gre; /* Enable GRE tunnel introspection */ | ||
int parse_6in4; /* Enable 6in4 tunnel introspection */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One single option int parse_tunnel
Hi Peter, we have another suggestion for tunneling support in sFlow, building on our previous pull request.
Let us know what you think! :)
Background
Motivated by pull request #559,
sfcapd
was recently extended with the capability to parse the nested headers used in GRE tunnels. Given the new command-line option-o gre
,sfcapd
will consider the innermost header stack (i.e., the tunneled flow) to be relevant for the stored flow information. In absence of that command-line option, the outermost header stack (i.e., the tunneling flow) is considered critical.Problem
While being a valuable first step, previous
sfcapd
tunnel functionality would benefit from the following two extensions:sfcapd
support for such IPv6 encapsulation would be desirable.Solution approach
We implement the extensions mentioned above as follows:
6in4
(i.e., with-o 6in4
) to convey that the parsing should advance beyond encapsulation headers.Result & Tests
For demonstations, we provide a PCAP file (extended with a 6in4 flow compared to pull request #559), containing sFlow records for 6 different flows that are encapsulated in an IP flow from 1.1.1.1 to 2.2.2.2. When recording these packets with
sfcapd
using-o gre,6in4
,nfdump
now shows the following records:(The third-to-last flow record comes from a pathological packet with an empty GRE payload).
Moreover, all tests run with
make check
still pass.Contributors
This pull request is proposed by Simon Scherrer from NetFabric.ai.