fix generated RouterOS script, clean up template

[j3n57h0m45]

After prologned testing of the generated MikroTik RouterOS script I became clear that the approach is not feasable and error prone for MikroTik devices such as the "hAP ac"

1. add ip to address-list
2. if it fails try to modify the respective entry in the address-list

Currently there is no efficient way to modify an ip address entry within an existing address-list (see also MikroTik Forum '/ip firewall address-list print where' is slow].

This only leaves us with this modified approach:

1. leave existing address-list untouched
2. add new ips to temporary address-list
3. swap out existing address-list with the updated address-list

@LaurenceJJones what do you think?

[LaurenceJJones]

After prologned testing of the generated MikroTik RouterOS script I became clear that the approach is not feasable and error prone for MikroTik devices such as the "hAP ac"

1. add ip to address-list

2. if it fails try to modify the respective entry in the address-list

Currently there is no efficient way to modify an ip address entry within an existing address-list (see also MikroTik Forum '/ip firewall address-list print where' is slow].

This only leaves us with this modified approach:

1. leave existing address-list untouched

2. add new ips to temporary address-list

3. swap out existing address-list with the updated address-list

@LaurenceJJones what do you think?

🤷🏻 I'm kind of relying on your knowledge on this part, if you say this is better then lets do it. I only released an RC so far so it hasnt propagated out to people yet, so lets make sure it optimal for all.

[j3n57h0m45]

So lets keep it at RC for the time being. It might come down to the point where we just have to admit that eg. MIPSBE Architechture in these devices is just not meant for any periodic real time processing of 25k entry lists :/

[j3n57h0m45]

Given the scope, sometimes the simplest solution is the most effective. When using on-device scripting, performance can be limited. Therefore, the original brute force method: deleting the address list and creating a new address list remains the most practical approach. By properly using the timeout parameter, we can avoid unnecessary logging events on the device, which helps to reduce the overall script execution time.

Testing Results with >22,000 entries

• hAP ac -> ~2 minutes
• RB5009 -> ~25 seconds

[j3n57h0m45]

Is there anything I can help with to get this merged?

[LaurenceJJones]

Is there anything I can help with to get this merged?

as long as you have tested and happy if there are blank lines then yeah we can merge.

[mmed]

Any plans moving forward with this?

In my case following command in existing implementation removes unrelated entries from my other address lists

/ip firewall address-list remove [ find list=$list address="$address" ]

[LaurenceJJones]

Any plans moving forward with this?

In my case following command in existing implementation removes unrelated entries from my other address lists

/ip firewall address-list remove [ find list=$list address="$address" ]

I'm not quite versed in the actual generated code, but if you review the changes and can confirm the new implementation will not cause this issue then we can merge as is?

AFAIK this should filter down to the list only? 🤷🏻

[mmed]

I'm not quite versed in the actual generated code, but if you review the changes and can confirm the new implementation will not cause this issue then we can merge as is?

I've built and published this PR to local container registry and been running for couple of days, so far works fine. 51K entries get processed in ~45 seconds (RB5009) and performant compared to current implementation (don't have the numbers ATM). I can confirm this does not touch other lists in my router.

Minor point; I'm not sure formatting scenario name (crowdsecurity/http-probing -> http-probing) should be part of this PR, one use case could be filtering by non crowdsecurity/ scenarios.

AFAIK this should filter down to the list only? 🤷🏻

That's correct, find subcommand ([ find list=$list address="$address" ]) should process with AND clause and I would expect not to touch other lists, but I wasn't able to figure out why this is happening.

[LaurenceJJones]

Great to hear it working!

On your point:

Minor point; I'm not sure formatting scenario name (crowdsecurity/http-probing -> http-probing) should be part of this PR, one use case could be filtering by non crowdsecurity/ scenarios.

I dont know why we needed to format the scenario, could it be that the / character which splits author and name causes an issue?

[mmed]

It's allowed to use / character inside comments so no issues with previous format.

For the record, second entry in the screenshot (manually added):

[mmed]

Latest changes seemed to break newlines

output with 8360705

/ip/firewall/address-list/remove [ find where list="blocklist_crowdsec" ];
:global CrowdSecAddIP;
:set CrowdSecAddIP do={
    :do { /ip/firewall/address-list/add list=blocklist_crowdsec address=$1 comment="$2" timeout=$3; } on-error={ }
}
$CrowdSecAddIP "1.1.145.149" "crowdsecurity/ssh-bf" "121h30m46s"$CrowdSecAddIP "1.11.103.45" "crowdsecurity/ssh-bf" "93h33m44s"

should be

/ip/firewall/address-list/remove [ find where list="blocklist_crowdsec" ];
:global CrowdSecAddIP;
:set CrowdSecAddIP do={
    :do { /ip/firewall/address-list/add list=blocklist_crowdsec address=$1 comment="$2" timeout=$3; } on-error={ }
}
$CrowdSecAddIP "1.1.145.149" "crowdsecurity/ssh-bf" "121h30m46s";
$CrowdSecAddIP "1.11.103.45" "crowdsecurity/ssh-bf" "93h33m44s";

[LaurenceJJones]

I fixed the whitespace issue, however, each line does not include a ; at the end (this has never been the case as far I can see.)

However, there is whitespace appearing in other places now

$CrowdSecAddIP "98.159.236.215" "crowdsecurity/ssh-slow-bf" "141h56m44s"

:set CrowdSecAddIP;
:set CrowdSecAddIPv6;

[mmed]

It works fine now, thanks!

Right, ; is not necessary (just a scripting habit): The end of the command line is represented by the token “;” or NEWLINE.

I don't think extra line breaks causes issue.