Project

General

Profile

Actions

Optimization #6572

open

Bug #5711: runmodes: Suricata does not hint anything about missing runmode

runmodes: fix `--list-runmodes` output

Added by Juliana Fajardini Reichow 7 months ago. Updated 25 days ago.

Status:
New
Priority:
Normal
Target version:
Effort:
Difficulty:
Label:
Beginner, Good First Issue

Description

As raised on https://github.com/OISF/suricata/pull/9743#discussion_r1386115590
the output of `--list-runmodes` isn't very useful at the moment.

We must consider what should be shown for that command.

Actions #1

Updated by Comfort Amaechi 7 months ago

If I may ask, what specific aspects of the current --list-runmodes output that you find lacking or could be improved?
Could you provide more details on the likely information or format you envision for the improved --list-runmodes output

Actions #2

Updated by Victor Julien 7 months ago

How about something like:

# suricata --list-runmodes

pcap:
  default runmode: autofp
  supported runmodes: single, autofp, workers
  description: libpcap based capture method
  notes: workers is only useful with NIC specific libpcap implementation supporting multi-capture (see XXX)
  yaml section: pcap
  commandline: --pcap or --pcap=<dev>
  supported features: none
af-packet:
  default runmode: workers
  supported runmodes: single, autofp, workers
  ...
  notes: on Linux -i uses af-packet by default
  supported features: ips csum_offload

Actions #3

Updated by Comfort Amaechi 7 months ago

Thank you for providing the suggested format. The proposed structure provides a clear and detailed overview of each runmode. Also Could you please provide insights on the preferred format for this output? For example, should it follow a tabular structure like the initial output or a different format?

Actions #4

Updated by Juliana Fajardini Reichow 7 months ago

Comfort Amaechi wrote in #note-3:

Thank you for providing the suggested format. The proposed structure provides a clear and detailed overview of each runmode. Also Could you please provide insights on the preferred format for this output? For example, should it follow a tabular structure like the initial output or a different format?

I would say follow the example Victor has given, I believe that's similar to what we currently have, right?

Actions #5

Updated by Lukas Sismis 4 months ago

  • Label Beginner, Good First Issue added
Actions #6

Updated by Gabriel Lima Luz 25 days ago

Hello, as I commented in the ticket #5711, I think it would be better to start working on this first. I would like to claim this ticket.
The Documentation contains information about the runmodes for me to base what the command message will say?

Actions #7

Updated by Juliana Fajardini Reichow 25 days ago

Gabriel Lima Luz wrote in #note-6:

Hello, as I commented in the ticket #5711, I think it would be better to start working on this first. I would like to claim this ticket.

Hello, indeed, this one should be finished first. Please, feel free to assign it to yourself :)

The Documentation contains information about the runmodes for me to base what the command message will say?

For starters, I would see what is the current output, then try to reformat it to follow Victor's suggestion on the comment above, and then you can share a draft PR and we can see what's missing.
For documentation, I'm afraid it may be a bit incomplete - if that's truly the case, this could be also part of this task, once the code part is done: updating the documentation, so all runmodes are at least listed there.

This would be what we have, for now: https://docs.suricata.io/en/latest/performance/runmodes.html

Actions #8

Updated by Gabriel Lima Luz 25 days ago

  • Assignee changed from OISF Dev to Gabriel Lima Luz
Actions #9

Updated by Gabriel Lima Luz 25 days ago ยท Edited

the function responsable to print the message is the RunModeRegisterRunModes in the rundmodes.c file :

void RunModeListRunmodes(void)
{
    printf("------------------------------------- Runmodes -------------------" 
           "-----------------------\n");

    printf("| %-17s | %-17s | %-10s \n",
           "RunMode Type", "Custom Mode ", "Description");
    printf("|-----------------------------------------------------------------" 
           "-----------------------\n");
    int i = RUNMODE_UNKNOWN + 1;
    int j = 0;
    for ( ; i < RUNMODE_USER_MAX; i++) {
        int mode_displayed = 0;
        for (j = 0; j < runmodes[i].cnt; j++) {
            if (mode_displayed == 1) {
                printf("|                   ----------------------------------------------" 
                       "-----------------------\n");
                RunMode *runmode = &runmodes[i].runmodes[j];
                printf("| %-17s | %-17s | %-27s \n",
                       "",
                       runmode->name,
                       runmode->description);
            } else {
                RunMode *runmode = &runmodes[i].runmodes[j];
                printf("| %-17s | %-17s | %-27s \n",
                       RunModeTranslateModeToName(runmode->runmode),
                       runmode->name,
                       runmode->description);
            }
            if (mode_displayed == 0)
                mode_displayed = 1;
        }
        if (mode_displayed == 1) {
            printf("|-----------------------------------------------------------------" 
                   "-----------------------\n");
        }
    }


from what I understand it prints info from a list of items of the type RunMode which is defined like this:
typedef struct RunMode_ {
    /* the runmode type */
    enum RunModes runmode;
    const char *name;
    const char *description;
    /* runmode function */
    int (*RunModeFunc)(void);
    int (*RunModeIsIPSEnabled)(void);
} RunMode;

So my question is to make the function follow the format suggested above what would be the best approach ? should I change the the struct fields and add something similar to the message or just change what the RunmMode->description contains ?

Actions #10

Updated by Juliana Fajardini Reichow 25 days ago

Gabriel Lima Luz wrote in #note-9:

So my question is to make the function follow the format suggested above what would be the best approach ? should I change the the struct fields and add something similar to the message or just change what the RunmMode->description contains ?

I think that what we can do is add more fields to the RunMode struct and populate these.
For a first PR, it is ok (imho) if you just provide a proposal of what the struct would look like, where/how these fields would be handled, and what the RunModeListRunmodes function would look like, and once that is settled, to move onto to populate these extra fields themselves.
So, steps and tasks that I see:
- proposing new structure fields to account for Victor's suggestion
- propose how/where those will be initialized
- propose what the RunModeListRunmodes will look like
- populate the approved new structure fields, for each run-mode
- update documentation as needed

(I may be missing stuff, but it's good to list what we already foresee)

Actions

Also available in: Atom PDF