Task #6355
closedTask #6308: detect/analyzer: add more keyword details
detect/analyzer: add more details for the tcp.mss keyword
Added by Juliana Fajardini Reichow about 1 year ago. Updated 7 months ago.
Description
Add more details to the tcp.mss keyword engine analysis output.
See what the tcp.mss keyword has on https://docs.suricata.io/en/latest/rules/header-keywords.html#tcp-mss
There are more general explanations in the parent task.
Updated by Juliana Fajardini Reichow about 1 year ago
- Copied from Task #6354: detect/analyzer: add more details for the tcp ack keyword added
Updated by Juliana Fajardini Reichow about 1 year ago
- Copied to Task #6356: detect/analyzer: add more details for the tcp.hdr keyword added
Updated by Daniel Olatunji about 1 year ago
Hello, I want to work on this.
But then, I'll be working on the same file (detect-engine-analyzer.c) I worked with on my last contribution, which has been approved but not merged yet. How do I handle it so there is no merge conflict when this is to be merged? Should I just add the previous edits I made before on this one too, by myself?
Updated by Juliana Fajardini Reichow about 1 year ago
Daniel Olatunji wrote in #note-3:
Hello, I want to work on this.
Ok, please, assign it to yourself, and thanks!
But then, I'll be working on the same file (detect-engine-analyzer.c) I worked with on my last contribution, which has been approved but not merged yet. How do I handle it so there is no merge conflict when this is to be merged? Should I just add the previous edits I made before on this one too, by myself?
Imho, the correct procedure here would be to create your new branch from master, and if the other PR gets merged before this new contribution, you would rebase your new work, and fix any existing conflicts.
(But you could also create your branch on top of the changes you've done, and then once those got merged and you rebased, they'd be skipped.)
Does that answer your question?
Updated by Daniel Olatunji about 1 year ago
Juliana Fajardini Reichow wrote in #note-4:
Daniel Olatunji wrote in #note-3:
Hello, I want to work on this.
Ok, please, assign it to yourself, and thanks!
But then, I'll be working on the same file (detect-engine-analyzer.c) I worked with on my last contribution, which has been approved but not merged yet. How do I handle it so there is no merge conflict when this is to be merged? Should I just add the previous edits I made before on this one too, by myself?
Imho, the correct procedure here would be to create your new branch from master, and if the other PR gets merged before this new contribution, you would rebase your new work, and fix any existing conflicts.
(But you could also create your branch on top of the changes you've done, and then once those got merged and you rebased, they'd be skipped.)
Does that answer your question?
What if I had already submitted a new PR before the previous one was merged?
Updated by Daniel Olatunji about 1 year ago
- Status changed from New to In Progress
- Assignee changed from Community Ticket to Daniel Olatunji
Updated by Juliana Fajardini Reichow about 1 year ago
Daniel Olatunji wrote in #note-5:
Juliana Fajardini Reichow wrote in #note-4:
Daniel Olatunji wrote in #note-3:
Hello, I want to work on this.
Ok, please, assign it to yourself, and thanks!
But then, I'll be working on the same file (detect-engine-analyzer.c) I worked with on my last contribution, which has been approved but not merged yet. How do I handle it so there is no merge conflict when this is to be merged? Should I just add the previous edits I made before on this one too, by myself?
Imho, the correct procedure here would be to create your new branch from master, and if the other PR gets merged before this new contribution, you would rebase your new work, and fix any existing conflicts.
(But you could also create your branch on top of the changes you've done, and then once those got merged and you rebased, they'd be skipped.)
Does that answer your question?
What if I had already submitted a new PR before the previous one was merged?
If you follow the first approach, there's nothing to be done, as this is a different keyword.
If you follow the second approach, you could mention on the new PR that you have built your work on top of previous work done on PR #6415
But considering this, I would say that it is indeed preferable that you take the first approach, therefore you don't have to link unrelated work in your new PR. It is not uncommon to have situations like this, and in such cases we will rebase and handle any possible merge conflicts that may occur - but I would expect this work not to lead to complex merge conflicts, as although it is the same file, you'll be adding a different keyword case.
Updated by Daniel Olatunji about 1 year ago
Juliana Fajardini Reichow wrote in #note-7:
Daniel Olatunji wrote in #note-5:
Juliana Fajardini Reichow wrote in #note-4:
Daniel Olatunji wrote in #note-3:
Hello, I want to work on this.
Ok, please, assign it to yourself, and thanks!
But then, I'll be working on the same file (detect-engine-analyzer.c) I worked with on my last contribution, which has been approved but not merged yet. How do I handle it so there is no merge conflict when this is to be merged? Should I just add the previous edits I made before on this one too, by myself?
Imho, the correct procedure here would be to create your new branch from master, and if the other PR gets merged before this new contribution, you would rebase your new work, and fix any existing conflicts.
(But you could also create your branch on top of the changes you've done, and then once those got merged and you rebased, they'd be skipped.)
Does that answer your question?
What if I had already submitted a new PR before the previous one was merged?
If you follow the first approach, there's nothing to be done, as this is a different keyword.
If you follow the second approach, you could mention on the new PR that you have built your work on top of previous work done on PR #6415
But considering this, I would say that it is indeed preferable that you take the first approach, therefore you don't have to link unrelated work in your new PR. It is not uncommon to have situations like this, and in such cases we will rebase and handle any possible merge conflicts that may occur - but I would expect this work not to lead to complex merge conflicts, as although it is the same file, you'll be adding a different keyword case.
OK then.
I understand now.
Thank you for your constant assistance :).
Updated by Daniel Olatunji about 1 year ago
I had been going down a never ending rabbit hole on this one, and it gets more confusing for me, since I am not very proficient in C.
I have been on it for a couple of days now, and cannot seem to figure out how to work with the "DetectU16Data".
I have looked through the file "detect-engine-uint.c" and it's corresponding header file.
Updated by Daniel Olatunji about 1 year ago
- Status changed from In Progress to In Review
PR for review(Suricata): https://github.com/OISF/suricata/pull/9673
PR for review(Suricata-Verify): https://github.com/OISF/suricata-verify/pull/1433
Updated by Philippe Antoine 7 months ago
- Status changed from In Review to Closed