Hi Geert,
On 2025/5/5 17:12, Geert Uytterhoeven wrote:
> Hi Tiwel,
>
> On Sat, 3 May 2025 at 07:17, Tiwei Bie <tiwei....@antgroup.com> wrote:
>> The only dependency on uml_net (i.e., the legacy network transport
>> infrastructure) is the call to uml_net_setup_etheraddr(). Implement
>> it inside vector to eliminate the uml_net dependency completely. It
>> will allow us to remove uml_net in the next step.
>>
>> Signed-off-by: Tiwei Bie <tiwei....@antgroup.com>
>
>
> Thanks for your patch!
>
>> --- a/arch/um/drivers/vector_kern.c
>> +++ b/arch/um/drivers/vector_kern.c
>> @@ -27,7 +27,6 @@
>> #include <init.h>
>> #include <irq_kern.h>
>> #include <irq_user.h>
>> -#include <net_kern.h>
>> #include <os.h>
>> #include "mconsole_kern.h"
>> #include "vector_user.h"
>> @@ -1539,7 +1538,56 @@ static void vector_timer_expire(struct timer_list *t)
>> napi_schedule(&vp->napi);
>> }
>>
>> +static void vector_setup_etheraddr(struct net_device *dev, char *str)
>> +{
>> + u8 addr[ETH_ALEN];
>> + char *end;
>> + int i;
>>
>> + if (str == NULL)
>> + goto random;
>> +
>> + for (i = 0; i < 6; i++) {
>> + addr[i] = simple_strtoul(str, &end, 16);
>> + if ((end == str) ||
>> + ((*end != ':') && (*end != ',') && (*end != '\0'))) {
>> + printk(KERN_ERR
>
> pr_err() (and friends, everywhere)
>
>> + "setup_etheraddr: failed to parse '%s' "
>> + "as an ethernet address\n", str);
>
> Please don't split error messages, as it makes it harder to
> grep for them (everywhere)
>
>> + goto random;
>> + }
>> + str = end + 1;
>> + }
>
> Can this use mac_pton()?
>
>> + if (is_multicast_ether_addr(addr)) {
>> + printk(KERN_ERR
>> + "Attempt to assign a multicast ethernet address to a "
>> + "device disallowed\n");
>> + goto random;
>> + }
>> + if (!is_valid_ether_addr(addr)) {
>> + printk(KERN_ERR
>> + "Attempt to assign an invalid ethernet address to a "
>> + "device disallowed\n");
>> + goto random;
>> + }
>> + if (!is_local_ether_addr(addr)) {
>> + printk(KERN_WARNING
>> + "Warning: Assigning a globally valid ethernet "
>> + "address to a device\n");
>> + printk(KERN_WARNING "You should set the 2nd rightmost bit in
>> "
>> + "the first byte of the MAC,\n");
>> + printk(KERN_WARNING "i.e. %02x:%02x:%02x:%02x:%02x:%02x\n",
>> + addr[0] | 0x02, addr[1], addr[2], addr[3], addr[4],
>> + addr[5]);
>
> Perhaps make a copy of addr[] with the bit set, so you can use %pM?
>
>> + }
>> + eth_hw_addr_set(dev, addr);
>> + return;
Thanks for the review! To keep changes minimal, vector_setup_etheraddr()
was copied from uml_net_setup_etheraddr() with only the name altered. This
series has been applied to the uml/next branch. I will submit a follow-up
patch to address your comments.
Regards,
Tiwei
>
> Gr{oetje,eeting}s,
>
> Geert
>
