[PATCH] trafgen: fix packet socket initialization with multiple CPUs

[Paolo Abeni]

The commit 78c13b71e196 ("trafgen: Allow to generate packets
to output pcap file") introduced a regression when output is
a network device and multiple CPU are in use: the packet
socket is created before fork() and thus the socket is shared
among all the processes: all of them except the first will
fail while setting the tx_ring.

Fix it splitting the io open() helper in a create() op,
called before forking, and the open() op called by each process.

Fixes: 78c13b71e196 ("trafgen: Allow to generate packets to output pcap file")
Signed-off-by: Paolo Abeni <pab...@redhat.com>
---
trafgen.c | 6 ++++--
trafgen_dev.c | 21 +++++++++++++--------
trafgen_dev.h | 4 +++-
3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/trafgen.c b/trafgen.c
index 9b54399..9c5a9a6 100644
--- a/trafgen.c
+++ b/trafgen.c
@@ -977,6 +977,7 @@ static void main_loop(struct ctx *ctx, char *confname, bool slow,
fflush(stdout);
}

+ dev_io_open(ctx->dev_out);
if (dev_io_is_netdev(ctx->dev_out) && ctx->qdisc_path == false)
set_sock_qdisc_bypass(dev_io_fd_get(ctx->dev_out), ctx->verbose);

@@ -1266,12 +1267,13 @@ int main(int argc, char **argv)
xlockme();

if (ctx.pcap_in) {
- ctx.dev_in = dev_io_open(ctx.pcap_in, DEV_IO_IN);
+ ctx.dev_in = dev_io_create(ctx.pcap_in, DEV_IO_IN);
if (!ctx.dev_in)
panic("Failed to open input device\n");
+ dev_io_open(ctx.dev_in);
}

- ctx.dev_out = dev_io_open(ctx.device, DEV_IO_OUT);
+ ctx.dev_out = dev_io_create(ctx.device, DEV_IO_OUT);
if (!ctx.dev_out)
panic("Failed to open output device\n");

diff --git a/trafgen_dev.c b/trafgen_dev.c
index f65442f..489da98 100644
--- a/trafgen_dev.c
+++ b/trafgen_dev.c
@@ -242,10 +242,11 @@ static const struct dev_io_ops dev_cfg_ops = {
.close = dev_cfg_close,
};

-struct dev_io *dev_io_open(const char *name, enum dev_io_mode_t mode)
+struct dev_io *dev_io_create(const char *name, enum dev_io_mode_t mode)
{
struct dev_io *dev = xzmalloc(sizeof(struct dev_io));

+ dev->mode = mode;
if (strstr(name, ".pcap")) {
dev->name = xstrdup(name);
dev->ops = &dev_pcap_ops;
@@ -260,16 +261,20 @@ struct dev_io *dev_io_open(const char *name, enum dev_io_mode_t mode)
return NULL;
}

- if (dev->ops->open) {
- if (dev->ops->open(dev, name, mode)) {
- xfree(dev);
- return NULL;
- }
- }
-
return dev;
};

+extern void dev_io_open(struct dev_io *dev)
+{
+ bug_on(!dev);
+ bug_on(!dev->ops);
+
+ if (dev->ops->open)
+ if (dev->ops->open(dev, dev->name, dev->mode))
+ panic("Cannot open io %s mode %d\n", dev->name,
+ dev->mode);
+}
+
int dev_io_write(struct dev_io *dev, struct packet *pkt)
{
bug_on(!dev);
diff --git a/trafgen_dev.h b/trafgen_dev.h
index 80086d7..bcb88f3 100644
--- a/trafgen_dev.h
+++ b/trafgen_dev.h
@@ -24,6 +24,7 @@ struct dev_io {
uint32_t pcap_magic;
bool is_initialized;
enum pcap_mode pcap_mode;
+ enum dev_io_mode_t mode;
size_t buf_len;
uint8_t *buf;

@@ -39,7 +40,8 @@ struct dev_io_ops {
void(*close) (struct dev_io *dev);
};

-extern struct dev_io *dev_io_open(const char *name, enum dev_io_mode_t mode);
+extern struct dev_io *dev_io_create(const char *name, enum dev_io_mode_t mode);
+extern void dev_io_open(struct dev_io *dev);
extern int dev_io_write(struct dev_io *dev, struct packet *pkt);
extern struct packet *dev_io_read(struct dev_io *dev);
extern int dev_io_ifindex_get(struct dev_io *dev);
--
2.13.5

[Vadim Kochan]

Thanks Paolo! Shame on me, I did not test it properly :(

[Paolo Abeni]

On Thu, 2017-09-14 at 10:40 +0300, Vadim Kochan wrote:
> Thanks Paolo! Shame on me, I did not test it properly :(

No problem at all, I guess. After all, bugs are the only feature shared
by all kind of software ;-)

I added the 'Fixes' tag following the kernel guidelines for patch
submission, to help 3rd party packager which can't rebase the their
version too often.

Cheers,

Paolo

[Tobias Klauser]

On 2017-09-13 at 17:54:52 +0200, Paolo Abeni <pab...@redhat.com> wrote:
> The commit 78c13b71e196 ("trafgen: Allow to generate packets
> to output pcap file") introduced a regression when output is
> a network device and multiple CPU are in use: the packet
> socket is created before fork() and thus the socket is shared
> among all the processes: all of them except the first will
> fail while setting the tx_ring.
>
> Fix it splitting the io open() helper in a create() op,
> called before forking, and the open() op called by each process.
>
> Fixes: 78c13b71e196 ("trafgen: Allow to generate packets to output pcap file")
> Signed-off-by: Paolo Abeni <pab...@redhat.com>

Applied, thanks a lot Paolo!

[Tobias Klauser]

On 2017-09-14 at 10:13:17 +0200, Paolo Abeni <pab...@redhat.com> wrote:
> On Thu, 2017-09-14 at 10:40 +0300, Vadim Kochan wrote:
> > Thanks Paolo! Shame on me, I did not test it properly :(
>
> No problem at all, I guess. After all, bugs are the only feature shared
> by all kind of software ;-)

Indeed :) I certainly also should have tested the fix more thoroughly.

> I added the 'Fixes' tag following the kernel guidelines for patch
> submission, to help 3rd party packager which can't rebase the their
> version too often.

This is very much appreciated.

Cheers
Tobias