net/netdev: Change SIOCSCANBITRATE to require interface down.

[csanchezdll]

Summary

In Linux, CAN bitrate is set with netlink, not ioctl, and you need to
set a bitrate before bringing the interface up (make sense). In Nuttx,
you can begin the interface up (it uses a default bitrate) and then
you can change it. My guess is calling ifup at the end is precisely to
avoid requiring another ifdown/ifup cycle from app code.

SocketCAN is a Linux thing, so we should try to mimic the behaviour.
Implementing netlink would be overkill (and moreover, it is common in
Linux to use libsocketcan to abstract the netlink internals anyways)
but at least we should have similar semantics

Impact

The change makes changing bitrate on an up interface fails. This is done on the portable, architecture-independent section of ioctl handling, so it affects all the drivers and makes the semantics clear.
Al existing SocketCAN drivers have also been updated.

Now the process for setting bitrate on an open interface is:

1. ifdown the interface is it's up
2. Call SIOCSCANBITRATE
3. ifup the interface.

This requires an update in apps/canutils/slcan. PR following.

Testing

The change was tested on a custom platform using s32k148 uC, confirming changing the bitrate no longer brings the interface up.

[nuttxpr]

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although some sections could be expanded for better clarity.

Strengths:

• Clear problem description: The summary clearly explains the difference between Linux SocketCAN and NuttX's current behavior regarding bitrate setting.
• Rationale for the change: The rationale for mimicking Linux behavior is well-justified.
• Impact description: The impact on users (requiring ifdown/ifup cycle) and drivers is stated.
• Testing mention: Testing is mentioned, including the platform used.
• Cross-referencing: Mention of a related PR in apps/canutils/slcan demonstrates consideration for the wider impact.

Areas for improvement:

• Conciseness in Summary: While the summary is clear, it could be slightly more concise by focusing directly on what the change does (preventing bitrate changes on an up interface) before explaining why.
• Missing details in Impact:
  • The impact on the build process, hardware, documentation, security, and compatibility are all marked as "NO" without explanation. Even if there is truly no impact, explicitly stating "NO - No changes to the build process" adds clarity. This prevents reviewers from wondering if these aspects were simply overlooked.
• Insufficient Testing Information: While testing is mentioned, it's lacking detail. The provided information doesn't show how the change was validated. The "testing logs before/after" sections are empty. Include specific commands used and the expected/observed output. For example:
  • Before: ifconfig can0 up; ip link set can0 type can bitrate 500000 (expected: bitrate change succeeds)
  • After: ifconfig can0 up; ip link set can0 type can bitrate 500000 (expected: operation fails with an error message)

Recommendation:

While the PR seems generally well-formed, adding the missing details in the Impact and Testing sections will significantly strengthen it and make the review process smoother. Provide concrete examples of commands used for testing and the expected outcomes, showcasing how the change functions and demonstrating its effectiveness.