weird redundant section behaviour #945

Closed
arr2036 opened this Issue Mar 31, 2015 · 3 comments

Projects

None yet

2 participants

@arr2036
Member
arr2036 commented Mar 31, 2015
(0)       # Executing section accounting from file /etc/raddb/sites-enabled/default
(0)         accounting {
(0)           redundant (null) {
(0)             redundant-load-balance group {
(0)             redundant-load-balance group {
(0) sql01: EXPAND %{tolower:type.%{Acct-Status-Type}.query}
(0) sql01:    --> type.start.query
(0) sql01: Using query template 'query'
rlm_sql (sql01): 0 of 0 connections in use.  You  may need to increase "spare"
rlm_sql (sql01): Opening additional connection (0), 1 of 32 pending slots used
rlm_sql_mysql: Starting connect to MySQL server
rlm_sql_mysql: Couldn't connect to MySQL server radius@mysql01:radius
rlm_sql_mysql: MySQL error: Access denied for user 'radius'@'192.168.239.156' (using password: YES)
rlm_sql_mysql: Socket destructor called, closing socket
rlm_sql (sql01): Opening connection failed (0)
(0)               [sql01] = fail
(0) sql02: EXPAND %{tolower:type.%{Acct-Status-Type}.query}
(0) sql02:    --> type.start.query
(0) sql02: Using query template 'query'
rlm_sql (sql02): 0 of 0 connections in use.  You  may need to increase "spare"
rlm_sql (sql02): Opening additional connection (0), 1 of 32 pending slots used
rlm_sql_mysql: Starting connect to MySQL server
rlm_sql_mysql: Couldn't connect to MySQL server radius@mysql02:radius
rlm_sql_mysql: MySQL error: Access denied for user 'radius'@'192.168.239.156' (using password: YES)
rlm_sql_mysql: Socket destructor called, closing socket
rlm_sql (sql02): Opening connection failed (0)
(0)               [sql02] = fail
(0) sql03: EXPAND %{tolower:type.%{Acct-Status-Type}.query}
(0) sql03:    --> type.start.query
(0) sql03: Using query template 'query'
rlm_sql (sql03): 0 of 0 connections in use.  You  may need to increase "spare"
rlm_sql (sql03): Opening additional connection (0), 1 of 32 pending slots used
rlm_sql_mysql: Starting connect to MySQL server
rlm_sql_mysql: Couldn't connect to MySQL server radius@mysql03:radius
rlm_sql_mysql: MySQL error: Access denied for user 'radius'@'192.168.239.156' (using password: YES)
rlm_sql_mysql: Socket destructor called, closing socket
rlm_sql (sql03): Opening connection failed (0)
(0)               [sql03] = fail
(0) sql02: EXPAND %{tolower:type.%{Acct-Status-Type}.query}
(0) sql02:    --> type.start.query
(0) sql02: Using query template 'query'
rlm_sql (sql02): 0 of 0 connections in use.  You  may need to increase "spare"
rlm_sql (sql02): Last connection attempt failed, waiting 30 seconds before retrying
(0)               [sql02] = fail
(0) sql03: EXPAND %{tolower:type.%{Acct-Status-Type}.query}
(0) sql03:    --> type.start.query
(0) sql03: Using query template 'query'
rlm_sql (sql03): 0 of 0 connections in use.  You  may need to increase "spare"
rlm_sql (sql03): Last connection attempt failed, waiting 30 seconds before retrying
(0)               [sql03] = fail
(0) sql03: EXPAND %{tolower:type.%{Acct-Status-Type}.query}
(0) sql03:    --> type.start.query
(0) sql03: Using query template 'query'
rlm_sql (sql03): 0 of 0 connections in use.  You  may need to increase "spare"
rlm_sql (sql03): Last connection attempt failed, waiting 30 seconds before retrying
(0)               [sql03] = fail
(0)             } # redundant-load-balance group = fail
(0) detail_session: EXPAND /var/log/radius/radacct/detail_session/detail-%Y%m%d:%H:%G
(0) detail_session:    --> /var/log/radius/radacct/detail_session/detail-20150331:18:02
(0) detail_session: /var/log/radius/radacct/detail_session/detail-%Y%m%d:%H:%G expands to /var/log/radius/radacct/detail_session/detail-20150331:18:02
(0) detail_session: EXPAND %t
(0) detail_session:    --> Tue Mar 31 18:02:19 2015
(0)             [detail_session] = ok
(0)           } # redundant (null) = ok
(0) attr_filter.accounting_response: EXPAND %{User-Name}
(0) attr_filter.accounting_response:    --> foo
(0) attr_filter.accounting_response: Matched entry DEFAULT at line 14
(0)           [attr_filter.accounting_response] = updated
(0)         } # accounting = updated

Instantiated

redundant-load-balance sql_session {
        sql01
        sql02
        sql03
}
        accounting {
                redundant {
                        sql_session
                        detail_session
                }

                attr_filter.accounting_response
        }

Works eventually but appears to loop over the sql modules too many times... Figured it might be a regression. This was with the v3.1.x branch.

@alandekok
Member

It's worse than that... it's calling:

sql01
sql02
sql03
sql02
sql03

it should be calling them in order. What's with:

a) calling them multiple times
b) skipping sql01?

@arr2036
Member
arr2036 commented Apr 4, 2015

I have a test case

# Check correct looping if all fail
update request {
    Tmp-Integer-2 := 0
    Tmp-Integer-3 := 0
    Tmp-Integer-4 := 0
    Tmp-Integer-5 := 0
}
redundant {
    redundant-load-balance {
        group {
            update request {
                Tmp-Integer-2 := "%{expr:&Tmp-Integer-2 + 1}"
            }
            fail
        }
        group {
            update request {
                Tmp-Integer-3 := "%{expr:&Tmp-Integer-3 + 1}"
            }
            fail
        }
        group {
            update request {
                Tmp-Integer-4 := "%{expr:&Tmp-Integer-4 + 1}"
            }
            fail
        }
        group {
            update request {
                Tmp-Integer-5 := "%{expr:&Tmp-Integer-5 + 1}"
            }
            fail
        }
    }
    ok
}

if (&Tmp-Integer-2 != 1) {
    update reply {
        Filter-Id += 'Fail 3a'
    }
}

if (&Tmp-Integer-3 != 1) {
    update reply {
        Filter-Id += 'Fail 3b'
    }
}

if (&Tmp-Integer-4 != 1) {
    update reply {
        Filter-Id += 'Fail 3c'
    }
}

if (&Tmp-Integer-5 != 1) {
    update reply {
        Filter-Id += 'Fail 3d'
    }
}

Stick it at the bottom of the redundant-load-balance tests.

Output with latest v3.0.x was:

(0)       update request {
(0)         Tmp-Integer-2 := 0
(0)         Tmp-Integer-3 := 0
(0)         Tmp-Integer-4 := 0
(0)         Tmp-Integer-5 := 0
(0)       } # update request = noop
(0)       redundant (null) {
(0)         redundant-load-balance group {
(0)         redundant-load-balance group {
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-3 + 1}
(0)                  --> 1
(0)               Tmp-Integer-3 := 1
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-4 + 1}
(0)                  --> 1
(0)               Tmp-Integer-4 := 1
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-5 + 1}
(0)                  --> 1
(0)               Tmp-Integer-5 := 1
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-4 + 1}
(0)                  --> 2
(0)               Tmp-Integer-4 := 2
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-5 + 1}
(0)                  --> 2
(0)               Tmp-Integer-5 := 2
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-5 + 1}
(0)                  --> 3
(0)               Tmp-Integer-5 := 3
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-2 + 1}
(0)                  --> 1
(0)               Tmp-Integer-2 := 1
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-3 + 1}
(0)                  --> 2
(0)               Tmp-Integer-3 := 2
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-4 + 1}
(0)                  --> 3
(0)               Tmp-Integer-4 := 3
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)           group {
(0)             update request {
(0)               EXPAND %{expr:&Tmp-Integer-5 + 1}
(0)                  --> 4
(0)               Tmp-Integer-5 := 4
(0)             } # update request = noop
(0)             [fail] = fail
(0)           } # group = fail
(0)         } # redundant-load-balance group = fail
(0)         [ok] = ok
(0)       } # redundant (null) = ok
(0)       if (&Tmp-Integer-2 != 1) {
(0)       if (&Tmp-Integer-2 != 1)  -> FALSE
(0)       if (&Tmp-Integer-3 != 1) {
(0)       if (&Tmp-Integer-3 != 1)  -> TRUE
(0)       if (&Tmp-Integer-3 != 1)  {
(0)         update reply {
(0)           Filter-Id += 'Fail 1b'
(0)         } # update reply = noop
(0)       } # if (&Tmp-Integer-3 != 1)  = noop
(0)       if (&Tmp-Integer-4 != 1) {
(0)       if (&Tmp-Integer-4 != 1)  -> TRUE
(0)       if (&Tmp-Integer-4 != 1)  {
(0)         update reply {
(0)           Filter-Id += 'Fail 1c'
(0)         } # update reply = noop
(0)       } # if (&Tmp-Integer-4 != 1)  = noop
(0)       if (&Tmp-Integer-5 != 1) {
(0)       if (&Tmp-Integer-5 != 1)  -> TRUE
(0)       if (&Tmp-Integer-5 != 1)  {
(0)         update reply {
(0)           Filter-Id += 'Fail 1d'
(0)         } # update reply = noop
(0)       } # if (&Tmp-Integer-5 != 1)  = noop
(0)       [pap] = updated
(0)     } # authorize = updated
(0)   Found Auth-Type = PAP
(0)   # Executing group from file src/tests/keywords//radiusd.conf
(0)     authenticate {
(0) pap: Login attempt with password
(0) pap: User authenticated successfully
(0)       [pap] = ok
(0)     } # authenticate = ok
(0) } # server default
(0) Virtual server sending reply
(0)   Filter-Id := 'filter'
(0)   Filter-Id += 'Fail 1b'
(0)   Filter-Id += 'Fail 1c'
(0)   Filter-Id += 'Fail 1d'
@arr2036
Member
arr2036 commented Apr 4, 2015

Still gets incorrect result without the outer redundant block

@alandekok alandekok added a commit that referenced this issue Apr 4, 2015
@alandekok alandekok Revert "Loop over COUNT entries. Maybe addresses #945"
Nope.

This reverts commit e774cb6.
435b836
@alandekok alandekok added a commit that referenced this issue Apr 4, 2015
@alandekok alandekok Fix for redundant-load-balance. Closes #945
In normal operations, modcall_child / modcall_recurse processes
the current node, and all of its children.  For redundant-load-balance,
we want to loop BACK from the end of the list to the start, AND
stop when we reach the first one we found again.

This means we have to tell the functions "process ONE node only",
and do all "next" operations ourselves.
b1236f0
@alandekok alandekok added a commit that referenced this issue Apr 4, 2015
@alandekok alandekok Revert "Loop over COUNT entries. Maybe addresses #945"
Nope.

This reverts commit e774cb6.
7e01921
@alandekok alandekok added a commit that closed this issue Apr 4, 2015
@alandekok alandekok Fix for redundant-load-balance. Closes #945
In normal operations, modcall_child / modcall_recurse processes
the current node, and all of its children.  For redundant-load-balance,
we want to loop BACK from the end of the list to the start, AND
stop when we reach the first one we found again.

This means we have to tell the functions "process ONE node only",
and do all "next" operations ourselves.
d363f5c
@alandekok alandekok closed this in d363f5c Apr 4, 2015
@jpereira jpereira added a commit to jpereira/freeradius-server that referenced this issue Apr 30, 2015
@alandekok @jpereira alandekok + jpereira Proper fix for #945. Auto-generate PW_AUTH_TYPE_* 5ed0071
@jpereira jpereira added a commit to jpereira/freeradius-server that referenced this issue May 5, 2015
@alandekok @jpereira alandekok + jpereira Proper fix for #945. Auto-generate PW_AUTH_TYPE_* 222b7a3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment