On 2015-04-14 at 01:10:34 +0200, Michal Purzynski <[email protected]>
wrote:
Please provide a short patch description and a Signed-off-by line (you
can use git commit -s to have git automatically add this for you). Also,
the subject line of the mail should start with [PATCH] and contain a
brief summary of your patch, I'd suggest something like
[PATCH] netsniff-ng: Implement fanout functionality
> ---
> netsniff-ng.c | 28 +++++++++++++++++++++++++---
> ring_rx.c | 17 ++++++++++++++++-
> ring_rx.h | 2 +-
> 3 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/netsniff-ng.c b/netsniff-ng.c
> index dfb99bb..c603919 100644
> --- a/netsniff-ng.c
> +++ b/netsniff-ng.c
> @@ -60,12 +60,13 @@ struct ctx {
> bool randomize, promiscuous, enforce, jumbo, dump_bpf, hwtimestamp,
> verbose;
> enum pcap_ops_groups pcap; enum dump_mode dump_mode;
> uid_t uid; gid_t gid; uint32_t link_type, magic;
> + uint32_t cluster_id, cluster_type;
Please use a tab to indent as in the lines above.
> };
>
> static volatile sig_atomic_t sigint = 0;
> static volatile bool next_dump = false;
>
> -static const char *short_options =
> "d:i:o:rf:MNJt:S:k:n:b:HQmcsqXlvhF:RGAP:Vu:g:T:DBU";
> +static const char *short_options =
> "d:i:o:rf:MNJt:S:k:n:b:HQmcsqXlvhF:RGAP:Vu:g:T:DBU:C:K:";
The colon after U shouldn't be there as -U/--update doesn't take an
argument.
> static const struct option long_options[] = {
> {"dev", required_argument, NULL, 'd'},
> {"in", required_argument, NULL, 'i'},
> @@ -81,6 +82,8 @@ static const struct option long_options[] = {
> {"user", required_argument, NULL, 'u'},
> {"group", required_argument, NULL, 'g'},
> {"magic", required_argument, NULL, 'T'},
> + {"fanout-group", required_argument, NULL, 'C'},
> + {"fanout-type", required_argument, NULL, 'K'},
Indent using tabs please.
> {"rand", no_argument, NULL, 'r'},
> {"rfraw", no_argument, NULL, 'R'},
> {"mmap", no_argument, NULL, 'm'},
> @@ -377,7 +380,7 @@ static void receive_to_xmit(struct ctx *ctx)
> bpf_dump_all(&bpf_ops);
> bpf_attach_to_sock(rx_sock, &bpf_ops);
>
> - ring_rx_setup(&rx_ring, rx_sock, size_in, ifindex_in, &rx_poll, false,
> ctx->jumbo, ctx->verbose);
> + ring_rx_setup(&rx_ring, rx_sock, size_in, ifindex_in, &rx_poll, false,
> ctx->jumbo, ctx->verbose, ctx->cluster_id, ctx->cluster_type);
Please add a line break to prevent the line from becoming too long.
Something like:
ring_rx_setup(&rx_ring, rx_sock, size_in, ifindex_in, &rx_poll, false,
ctx->jumbo, ctx->verbose, ctx->cluster_id, ctx->cluster_type);
would be fine.
> ring_tx_setup(&tx_ring, tx_sock, size_out, ifindex_out, ctx->jumbo,
> ctx->verbose);
>
> dissector_init_all(ctx->print_mode);
> @@ -925,7 +928,7 @@ static void recv_only_or_dump(struct ctx *ctx)
> printf("HW timestamping enabled\n");
> }
>
> - ring_rx_setup(&rx_ring, sock, size, ifindex, &rx_poll,
> is_defined(HAVE_TPACKET3), true, ctx->verbose);
> + ring_rx_setup(&rx_ring, sock, size, ifindex, &rx_poll,
> is_defined(HAVE_TPACKET3), true, ctx->verbose, ctx->cluster_id,
> ctx->cluster_type);
Same here, please add a line break.
> dissector_init_all(ctx->print_mode);
>
> @@ -1358,6 +1361,25 @@ int main(int argc, char **argv)
> case 'h':
> help();
> break;
> + case 'C':
> + ctx.cluster_id = (uint32_t) strtoul(optarg, NULL, 0);
> + break;
> + case 'K':
> + if (!strncmp(optarg, "hash", strlen("hash")))
> + ctx.cluster_type = PACKET_FANOUT_HASH;
> + else if (!strncmp(optarg, "lb", strlen("lb")))
> + ctx.cluster_type = PACKET_FANOUT_LB;
> + else if (!strncmp(optarg, "cpu", strlen("cpu")))
> + ctx.cluster_type = PACKET_FANOUT_CPU;
> + else if (!strncmp(optarg, "rnd", strlen("rnd")))
> + ctx.cluster_type = PACKET_FANOUT_RND;
> + else if (!strncmp(optarg, "roll", strlen("roll")))
> + ctx.cluster_type = PACKET_FANOUT_ROLLOVER;
> +#ifdef PACKET_FANOUT_QM
> + else if (!strncmp(optarg, "qm", strlen("qm")))
> + ctx.cluster_type = PACKET_FANOUT_QM;
> +#endif
> + break;
Again, the indenting should be done with tabs here. Same goes for the
rest of the patch.
> case '?':
> switch (optopt) {
> case 'd':
> diff --git a/ring_rx.c b/ring_rx.c
> index 8ad64d1..c97dd2d 100644
> --- a/ring_rx.c
> +++ b/ring_rx.c
> @@ -209,9 +209,23 @@ static void alloc_rx_ring_frames(int sock, struct ring
> *ring)
> rx_ring_get_size(ring, v3));
> }
>
> +void create_cluster(int sock, uint32_t cluster_id, uint32_t cluster_mode)
> +{
> + uint32_t cluster_option = 0;
> + int ret = 0;
> +
> + if (cluster_mode == PACKET_FANOUT_HASH)
> + cluster_mode = PACKET_FANOUT_HASH | PACKET_FANOUT_FLAG_DEFRAG;
> + else
> + cluster_option = (cluster_id | (cluster_mode << 16));
> + ret = setsockopt(sock, SOL_PACKET, PACKET_FANOUT,(void
> *)&cluster_option, sizeof(cluster_option));
The explicit void* cast is not necessary here.
> + if (ret < 0)
> + panic("Cannot set fanout ring mode!\n");
> +}
> +
> void ring_rx_setup(struct ring *ring, int sock, size_t size, int ifindex,
> struct pollfd *poll, bool v3, bool jumbo_support,
> - bool verbose)
> + bool verbose, uint32_t cluster_id, uint32_t cluster_type)
> {
> fmemset(ring, 0, sizeof(*ring));
> setup_rx_ring_layout(sock, ring, size, jumbo_support, v3);
> @@ -220,6 +234,7 @@ void ring_rx_setup(struct ring *ring, int sock, size_t
> size, int ifindex,
> alloc_rx_ring_frames(sock, ring);
> bind_ring_generic(sock, ring, ifindex, false);
> prepare_polling(sock, poll);
> + create_cluster(sock, cluster_id, cluster_type);
This function shouldn't be called unconditionally. I'd rather call this
only if the user specified cluster id/type on the command line.
> }
>
> void sock_rx_net_stats(int sock, unsigned long seen)
> diff --git a/ring_rx.h b/ring_rx.h
> index edd0feb..ae2cd76 100644
> --- a/ring_rx.h
> +++ b/ring_rx.h
> @@ -13,7 +13,7 @@
>
> extern void ring_rx_setup(struct ring *ring, int sock, size_t size, int
> ifindex,
> struct pollfd *poll, bool v3, bool jumbo_support,
> - bool verbose);
> + bool verbose, uint32_t cluster_id, uint32_t
> cluster_type);
> extern void destroy_rx_ring(int sock, struct ring *ring);
> extern void sock_rx_net_stats(int sock, unsigned long seen);
>
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "netsniff-ng" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups
"netsniff-ng" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
