CANopenLinux icon indicating copy to clipboard operation
CANopenLinux copied to clipboard

setRxFilters() tries to set more RX filters than the kernel allows, resulting in CO_ERROR_SYSCALL.

Open maservant opened this issue 9 months ago • 0 comments

We are working on a project with a "meta-OD" that would configure individual RPDOs dynamically according to the initialisation function of a library that is loaded at runtime. This requires that all possible valid RPDOs have their communication and mapping parameters preset and disabled, but the CO_CANsetNormalMode() function still tries to load RX filters for those packets (which makes sense, any arbitrary RPDO could be enabled or disabled at runtime and the CAN socket should accept any packet that we could be interested in down the road).

The issue is that the CO_CANmodule_t documentation in CO_driver.h makes it clear that

    volatile bool_t useCANrxFilters;   /**< Value different than zero indicates,
            that CAN module hardware filters are used for CAN reception. If
            there is not enough hardware filters, they won't be used. In this
            case will be *all* received CAN messages processed by software. */

The actual code in the CANopenLinux CO_driver.c, however, seems to skip this check, resulting in a CO_ERROR_SYSCALL.

/** Set up or update socketCAN rx filters *************************************/
static CO_ReturnError_t setRxFilters(CO_CANmodule_t *CANmodule)
{
    size_t i;
    int count;
    CO_ReturnError_t retval;

    struct can_filter rxFiltersCpy[CANmodule->rxSize];

    count = 0;
    /* remove unused entries ( id == 0 and mask == 0 ) as they would act as
     * "pass all" filter */
    for (i = 0; i < CANmodule->rxSize; i ++) {
        if ((CANmodule->rxFilter[i].can_id != 0) ||
            (CANmodule->rxFilter[i].can_mask != 0)) {

            rxFiltersCpy[count] = CANmodule->rxFilter[i];

            count ++;
        }
    }

    if (count == 0) {
        /* No filter is set, disable RX */
        return disableRx(CANmodule);
    }

    retval = CO_ERROR_NO;
    for (i = 0; i < CANmodule->CANinterfaceCount; i ++) {
      int ret = setsockopt(CANmodule->CANinterfaces[i].fd, SOL_CAN_RAW, CAN_RAW_FILTER,
                       rxFiltersCpy, sizeof(struct can_filter) * count);
      if(ret < 0){
          log_printf(LOG_ERR, CAN_FILTER_FAILED,
                     CANmodule->CANinterfaces[i].ifName);
          log_printf(LOG_DEBUG, DBG_ERRNO, "setsockopt()");
          retval = CO_ERROR_SYSCALL;
      }
    }

    return retval;
}

On my test system (Debian Bookworm running in VirtualBox), the limit is 512. This is accessible through the CAN_RAW_FILTER_MAX variable in <linux/can.h>. We would need to add a check to prevent calling setsockopt() with an oversize filter list, and revert to no kernel filtering if the filter list would be oversize. This would ensure the behaviour of the driver matches the documentation, and in any case slow performance is better than a syscall error and packet loss.

maservant avatar Apr 30 '24 17:04 maservant