MadaMzandu / uisp-ros-plugin

This is a follow up replacement for the previous Uisp api for mikrotik routers.
32 stars 9 forks source link

compatibility with ros7 #40

Closed kostopravby closed 1 year ago

kostopravby commented 1 year ago

does this plugin works with mikrotik ros7? block list is empty when I suspend service. the plugin removes dhcp lease and shaping, but doesn't add to block list and also adds shaping with the same speed(not with speed from settings for blocked clients). also static lease doesn't have correct description when I added Mac address and Nas(custom attribute for device name)attributes to service.

MadaMzandu commented 1 year ago

Thanks for feedback sounds like multiple problems. The plugin should be compatible with Ros7. Let me quickly verify the problems

MadaMzandu commented 1 year ago
  1. Disabled rate - Confirmed there is a bug with disabled rate. Will fix shortly.

  2. Block list - this appear to be working - address is added to block list when lease is acquired

  3. Description - Is it possible to send screenshot of description after custom attributes were added

Thanks

MadaMzandu commented 1 year ago

Ok release has been updated with disabled rate fix.

Have verified block list on ROS 7.10 and it works, but client needs to acquire lease.

Will wait for more info on the bad lease description

Thanks

pashtete commented 1 year ago
  1. bug in lease time. was configured 10 minutes. changed some leases to 60 minutes
  2. ip exclusions - ip 10.20.1.1 added. in the pool - 10.20.1.0/30. had to add anything else? bcs used 1.1-1.4. customers stopped to work.
  3. already existing static leases were changed. all of them. 90% of customer devices - 3rd party. then need to check/change leases back or devices in uisp. maybe do no change leases and just read the address? i mean it can be a problem if somebody else will start to use this plugin for already working mikrotik.
  4. didnt understand logic of ipv4 pools. added 2 - /24 and /30. leases changed and messed. see p.2 and p.3
  5. about block list. i will add info later thanks
MadaMzandu commented 1 year ago

Thanks for the update reviewing the problems indicated.

  1. If you have defined an IP address attribute, you can specify a fixed ip address for each client and the plugin will not assign. Otherwise your suggestion may be a problem if you actually want to change the clients address
MadaMzandu commented 1 year ago

Hi,

Thanks for valuable feedback. The problems indicated have be fixed in reupped version. https://github.com/MadaMzandu/uisp-ros-plugin/releases/tag/2.0.2.3

  1. the lease time setting was being ignored and defaulting to 60 minutes
  2. the ip exclusions check for single addresses was causing an error.
pashtete commented 1 year ago

thank you.

so in our case then better to use fixed ip attribute. got it

pashtete commented 1 year ago

hi

first of all - lease time setting is working correctly now. thx.

i assigned to all existing customers fixed ips. leases are active and waiting. my test connection have Dynamic lease now. do i have to make static lease or plugin should do it by itself? as i see by log last one. but for some reason plugin trying to use used ip. serivice already Suspended added to attributes mac and router name. dynamic ip 10.20.0.54. trying to assign 10.20.0.47 which already assigned to another customer. 2023-06-19 23:21:54.682: mt write error: [{"batch":95226,"path":"\/ip\/dhcp-server\/lease","address":"10.20.0.47","mac-address":"74:4D:28:4E:BF:14","insert-queue-before":"bottom","address-lists":"block_clients","lease-time":"10m","comment":"2638 - CUSTOMER NAME - 11752"},{"!trap":[{"message":"failure: already have static lease with this IP address"}]}] and this error is coming to Jobs also

When i remove suspension - this dynamic lease was deleted from router. wait when it come back, edit/save service: 2023-06-19 23:33:42.207: mt write error: [{"batch":174999,"path":"\/ip\/dhcp-server\/lease","address":"10.20.0.47","mac-address":"74:4D:28:4E:BF:14","insert-queue-before":"bottom","address-lists":"active_clients","lease-time":"10m","comment":"2638 - CUSTOMER NAME - 11752"},{"!trap":[{"message":"failure: can not change dynamic lease"}]}]

when i add fixed ip to attribute, same as dynamic lease: 2023-06-19 23:35:29.775: mt write error: [{"batch":109879,"path":"\/ip\/dhcp-server\/lease","address":"10.20.0.54","mac-address":"74:4D:28:4E:BF:14","insert-queue-before":"bottom","address-lists":"active_clients","lease-time":"10m","comment":"2638 - CUSTOMER NAME - 11752"},{"!trap":[{"message":"failure: can not change dynamic lease"}]}]

changed manually lease to static. edit/save service - no any errors or new messages in log.

Suspend the service. plugin delete static lease. add new static lease with same ip/mac and Server field - ALL. lease is not active - waiting. this ip has been added to disabled address list when customer's router renewed dhcp lease.

Unsuspend the service. plugin delete static lease again. add new static lease with same ip/mac and Server field - ALL. lease is not active - waiting. this ip has been removed from disabled address list.

My question is - is it possible do not delete lease and add/remove ip to/from disabled list? bcs need to wait renew of lease. with 10minutes its not a problem. with higher lease time - problem. or is this behavior expected?

Thank you

MadaMzandu commented 1 year ago

Thanks for feedback again. These are all very good observations.

  1. I will modify the code to preserve the previous lease unless it is absolutely required to delete the previous lease.
  2. I will see if it is possible to "make static" if a dynamic lease is found.

Otherwise it is recommended to use a short lease time to ensure that the client is updated quickly the mikrotik default of 10 minutes is a good setting.

I will update when a new build is ready for you to test

MadaMzandu commented 1 year ago

Hi The suggested improvements have been applied, please test and update.

https://github.com/MadaMzandu/uisp-ros-plugin/releases/tag/2.0.2.4

  1. existing dynamic lease will be converted to static,
  2. existing lease will be preserved on edit/suspend/unsuspend - except if client's mac address/nas has changed
pashtete commented 1 year ago

hi sorry for delay

  1. All cleared. again same router as a "new" customer. Dynamic lease 10.20.0.54. add just MAC and NAS name, without Fixed IP first:

    • dynamic lease converted to static(*)
    • queue added with wrong Target IP - again 10.20.0.47. i think this mac or ip stuck in DB somewhere, will try with new mac later
  2. All cleared again Dynamic lease 10.20.0.54. add MAC and NAS name and Fixed IP 10.20.0.65:

    • all good. lease converted to static and with IP from attribute.
    • queue is correct too
  3. Suspend/Unsuspend:

    • lease not deleted
    • changes of address lists are correctly
    • queue changed to correct speed limits All good
  4. End Service:

    • lease deleted
    • queue deleted

in general I have some doubts which is not for this "issue"

Thank you

MadaMzandu commented 1 year ago

Thank you for the update. I think for (1) this normal behavior, the plugin will not reuse the mikrotik assigned address. This means if you have a /24 you can let the mikrotik use a /25 and the plugin use another /25 without conflicts.

Let me know if there are other issues.