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

App-layer plugin preliminary work 5053 v2.4 #11426

Closed

Conversation

catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
Preliminary work for https://redmine.openinfosecfoundation.org/issues/5053

Describe changes:

  • get ready to use dynamic number of app-layer protos for some global arrays : run modes and output

Small PR good in itself.

#11373 next round
Based on #11425 to get green CI :-p

Still more work to do : I guess stack allocated arrays are fine, but the global variables cf git grep '\[ALPROTO_MAX' in

  • app-layer-detect-proto.c
  • app-layer-frames.c
  • app-layer-parser.c
  • app-layer-protos.c
  • app-layer.c
    need to be allocated and freed, with taking care of the initialization order, so that we know ALPROTO_MAX final value...

THashInitConfig may not allocate array and increase memuse.
Such a failure leads to THashShutdown which should not decrease
the memuse.

Ticket: 7135
@catenacyber catenacyber requested a review from victorjulien as a code owner July 4, 2024 13:37
@@ -917,7 +972,7 @@ static int JsonGenericLogger(ThreadVars *tv, void *thread_data, const Packet *p,
}

const char *name;
switch (al->proto) {
switch (f->proto) {
Copy link
Member

Choose a reason for hiding this comment

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

instead of doing this at runtime, can we do it at init time and store the result in EveJsonSimpleAppLayerLogger?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice idea :-)

FatalError("Failed to allocate simple_json_applayer_loggers");
}
// ALPROTO_HTTP1 special: uses some options flags
RegisterSimpleJsonApplayerLogger(ALPROTO_FTP, EveFTPLogCommand);
Copy link
Member

Choose a reason for hiding this comment

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

can we move these into the respective parsers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the end, we want to.

But this is not obvious now : when the respective parsers register themselves, we do not know yet ALPROTO_MAX

So, I would like to keep this for a later PR...

@catenacyber
Copy link
Contributor Author

Continued in #11429

@catenacyber catenacyber closed this Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants