| Message ID | 1417450537-7640-4-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 01/12/14 16:15, Maxim Uvarov wrote: > @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) > else > return 0; > } > + > +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char *mac_addr, > + size_t addr_size) > +{ > + pktio_entry_t *entry; > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (addr_size != ETH_ALEN) > + return -ENOMEM; What about EUI-64 MAC addresses? Do we need/want to care about them? > + > + entry = get_entry(id); > + if (entry == NULL) { > + ODP_DBG("pktio entry %d does not exist\n", id); > + return -ENOENT; > + } > + > + if (entry->s.pkt_sock_mmap.sockfd > -1) > + sockfd = entry->s.pkt_sock_mmap.sockfd; > + else > + sockfd = entry->s.pkt_sock.sockfd; > + > + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); > + ifr.ifr_name[IFNAMSIZ - 1] = 0; > + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); > + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; > + > + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); > + if (ret < 0) { > + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); > + return ret; > + } > + > + return 0; > +} > + > +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, > + size_t addr_size) > +{ > + pktio_entry_t *entry; > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (addr_size < ETH_ALEN) > + return -ENOMEM; > + > + entry = get_entry(id); > + if (entry == NULL) { > + ODP_DBG("pktio entry %d does not exist\n", id); > + return -ENOENT; > + } > + > + if (entry->s.pkt_sock_mmap.sockfd > -1) > + sockfd = entry->s.pkt_sock_mmap.sockfd; > + else > + sockfd = entry->s.pkt_sock.sockfd; > + > + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); > + ifr.ifr_name[IFNAMSIZ - 1] = 0; > + > + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); > + if (ret < 0) { > + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); > + return ret; > + } > + > + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, > + ETH_ALEN); > + > + return ETH_ALEN; > +} >
On 12/02/2014 08:11 PM, Zoltan Kiss wrote: > > > On 01/12/14 16:15, Maxim Uvarov wrote: >> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >> else >> return 0; >> } >> + >> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >> *mac_addr, >> + size_t addr_size) >> +{ >> + pktio_entry_t *entry; >> + int sockfd; >> + struct ifreq ifr; >> + int ret; >> + >> + if (addr_size != ETH_ALEN) >> + return -ENOMEM; > What about EUI-64 MAC addresses? Do we need/want to care about them? I did linux-generic implementation for hw which I have. EUI-64 can be added with other patch which we should test first. Can EUI-64 be set up with the same ioctl or something else is needed? I need to take a look at it separately. Maxim. > >> + >> + entry = get_entry(id); >> + if (entry == NULL) { >> + ODP_DBG("pktio entry %d does not exist\n", id); >> + return -ENOENT; >> + } >> + >> + if (entry->s.pkt_sock_mmap.sockfd > -1) >> + sockfd = entry->s.pkt_sock_mmap.sockfd; >> + else >> + sockfd = entry->s.pkt_sock.sockfd; >> + >> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >> + >> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >> + if (ret < 0) { >> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >> + size_t addr_size) >> +{ >> + pktio_entry_t *entry; >> + int sockfd; >> + struct ifreq ifr; >> + int ret; >> + >> + if (addr_size < ETH_ALEN) >> + return -ENOMEM; >> + >> + entry = get_entry(id); >> + if (entry == NULL) { >> + ODP_DBG("pktio entry %d does not exist\n", id); >> + return -ENOENT; >> + } >> + >> + if (entry->s.pkt_sock_mmap.sockfd > -1) >> + sockfd = entry->s.pkt_sock_mmap.sockfd; >> + else >> + sockfd = entry->s.pkt_sock.sockfd; >> + >> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >> + >> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >> + if (ret < 0) { >> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >> + return ret; >> + } >> + >> + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >> + ETH_ALEN); >> + >> + return ETH_ALEN; >> +} >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 2 December 2014 at 21:37, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/02/2014 08:11 PM, Zoltan Kiss wrote: >> >> >> >> On 01/12/14 16:15, Maxim Uvarov wrote: >>> >>> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >>> else >>> return 0; >>> } >>> + >>> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >>> *mac_addr, >>> + size_t addr_size) >>> +{ >>> + pktio_entry_t *entry; >>> + int sockfd; >>> + struct ifreq ifr; >>> + int ret; >>> + >>> + if (addr_size != ETH_ALEN) >>> + return -ENOMEM; >> >> What about EUI-64 MAC addresses? Do we need/want to care about them? Yes why such a restrictive check? Why not just "if (addr_size < ETH_ALEN)" above? I can't see why it should be an error to provide a buffer that is larger than absolutely necessary. -- Ola > > > I did linux-generic implementation for hw which I have. EUI-64 can be added > with other > patch which we should test first. Can EUI-64 be set up with the same ioctl > or something else > is needed? I need to take a look at it separately. > > Maxim. > > >> >>> + >>> + entry = get_entry(id); >>> + if (entry == NULL) { >>> + ODP_DBG("pktio entry %d does not exist\n", id); >>> + return -ENOENT; >>> + } >>> + >>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>> + else >>> + sockfd = entry->s.pkt_sock.sockfd; >>> + >>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >>> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >>> + >>> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >>> + if (ret < 0) { >>> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >>> + size_t addr_size) >>> +{ >>> + pktio_entry_t *entry; >>> + int sockfd; >>> + struct ifreq ifr; >>> + int ret; >>> + >>> + if (addr_size < ETH_ALEN) >>> + return -ENOMEM; >>> + >>> + entry = get_entry(id); >>> + if (entry == NULL) { >>> + ODP_DBG("pktio entry %d does not exist\n", id); >>> + return -ENOENT; >>> + } >>> + >>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>> + else >>> + sockfd = entry->s.pkt_sock.sockfd; >>> + >>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>> + >>> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >>> + if (ret < 0) { >>> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >>> + return ret; >>> + } >>> + >>> + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >>> + ETH_ALEN); >>> + >>> + return ETH_ALEN; >>> +} >>> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 12/02/2014 11:44 PM, Ola Liljedahl wrote: > On 2 December 2014 at 21:37, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> On 12/02/2014 08:11 PM, Zoltan Kiss wrote: >>> >>> >>> On 01/12/14 16:15, Maxim Uvarov wrote: >>>> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >>>> else >>>> return 0; >>>> } >>>> + >>>> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >>>> *mac_addr, >>>> + size_t addr_size) >>>> +{ >>>> + pktio_entry_t *entry; >>>> + int sockfd; >>>> + struct ifreq ifr; >>>> + int ret; >>>> + >>>> + if (addr_size != ETH_ALEN) >>>> + return -ENOMEM; >>> What about EUI-64 MAC addresses? Do we need/want to care about them? > Yes why such a restrictive check? > Why not just "if (addr_size < ETH_ALEN)" above? > I can't see why it should be an error to provide a buffer that is > larger than absolutely necessary. > > -- Ola Application should request valid size, isn't it? I.e. 6 or 8. What is the use to request more then can be returned? Maxim. >> >> I did linux-generic implementation for hw which I have. EUI-64 can be added >> with other >> patch which we should test first. Can EUI-64 be set up with the same ioctl >> or something else >> is needed? I need to take a look at it separately. >> >> Maxim. >> >> >>>> + >>>> + entry = get_entry(id); >>>> + if (entry == NULL) { >>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>> + return -ENOENT; >>>> + } >>>> + >>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>> + else >>>> + sockfd = entry->s.pkt_sock.sockfd; >>>> + >>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >>>> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >>>> + >>>> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >>>> + if (ret < 0) { >>>> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >>>> + return ret; >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >>>> + size_t addr_size) >>>> +{ >>>> + pktio_entry_t *entry; >>>> + int sockfd; >>>> + struct ifreq ifr; >>>> + int ret; >>>> + >>>> + if (addr_size < ETH_ALEN) >>>> + return -ENOMEM; >>>> + >>>> + entry = get_entry(id); >>>> + if (entry == NULL) { >>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>> + return -ENOENT; >>>> + } >>>> + >>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>> + else >>>> + sockfd = entry->s.pkt_sock.sockfd; >>>> + >>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>> + >>>> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >>>> + if (ret < 0) { >>>> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >>>> + return ret; >>>> + } >>>> + >>>> + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >>>> + ETH_ALEN); >>>> + >>>> + return ETH_ALEN; >>>> +} >>>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
On 2 December 2014 at 21:55, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/02/2014 11:44 PM, Ola Liljedahl wrote: >> >> On 2 December 2014 at 21:37, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>> >>> On 12/02/2014 08:11 PM, Zoltan Kiss wrote: >>>> >>>> >>>> >>>> On 01/12/14 16:15, Maxim Uvarov wrote: >>>>> >>>>> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >>>>> else >>>>> return 0; >>>>> } >>>>> + >>>>> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >>>>> *mac_addr, >>>>> + size_t addr_size) >>>>> +{ >>>>> + pktio_entry_t *entry; >>>>> + int sockfd; >>>>> + struct ifreq ifr; >>>>> + int ret; >>>>> + >>>>> + if (addr_size != ETH_ALEN) >>>>> + return -ENOMEM; >>>> >>>> What about EUI-64 MAC addresses? Do we need/want to care about them? >> >> Yes why such a restrictive check? >> Why not just "if (addr_size < ETH_ALEN)" above? >> I can't see why it should be an error to provide a buffer that is >> larger than absolutely necessary. >> >> -- Ola > > > Application should request valid size, isn't it? I.e. 6 or 8. What is the > use to request more then can be returned? The application might not know or care exactly which size a MAC address has on a specific device. You are just making the application more complicated and I can't see any benefits. -- Ola > > Maxim. > >>> >>> I did linux-generic implementation for hw which I have. EUI-64 can be >>> added >>> with other >>> patch which we should test first. Can EUI-64 be set up with the same >>> ioctl >>> or something else >>> is needed? I need to take a look at it separately. >>> >>> Maxim. >>> >>> >>>>> + >>>>> + entry = get_entry(id); >>>>> + if (entry == NULL) { >>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>> + return -ENOENT; >>>>> + } >>>>> + >>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>> + else >>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>> + >>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >>>>> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >>>>> + >>>>> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >>>>> + if (ret < 0) { >>>>> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >>>>> + size_t addr_size) >>>>> +{ >>>>> + pktio_entry_t *entry; >>>>> + int sockfd; >>>>> + struct ifreq ifr; >>>>> + int ret; >>>>> + >>>>> + if (addr_size < ETH_ALEN) >>>>> + return -ENOMEM; >>>>> + >>>>> + entry = get_entry(id); >>>>> + if (entry == NULL) { >>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>> + return -ENOENT; >>>>> + } >>>>> + >>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>> + else >>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>> + >>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>> + >>>>> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >>>>> + if (ret < 0) { >>>>> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + memcpy(mac_addr, (unsigned char >>>>> *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >>>>> + ETH_ALEN); >>>>> + >>>>> + return ETH_ALEN; >>>>> +} >>>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 12/03/2014 12:42 AM, Ola Liljedahl wrote: > On 2 December 2014 at 21:55, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> On 12/02/2014 11:44 PM, Ola Liljedahl wrote: >>> On 2 December 2014 at 21:37, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>>> On 12/02/2014 08:11 PM, Zoltan Kiss wrote: >>>>> >>>>> >>>>> On 01/12/14 16:15, Maxim Uvarov wrote: >>>>>> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >>>>>> else >>>>>> return 0; >>>>>> } >>>>>> + >>>>>> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >>>>>> *mac_addr, >>>>>> + size_t addr_size) >>>>>> +{ >>>>>> + pktio_entry_t *entry; >>>>>> + int sockfd; >>>>>> + struct ifreq ifr; >>>>>> + int ret; >>>>>> + >>>>>> + if (addr_size != ETH_ALEN) >>>>>> + return -ENOMEM; >>>>> What about EUI-64 MAC addresses? Do we need/want to care about them? >>> Yes why such a restrictive check? >>> Why not just "if (addr_size < ETH_ALEN)" above? >>> I can't see why it should be an error to provide a buffer that is >>> larger than absolutely necessary. >>> >>> -- Ola >> >> Application should request valid size, isn't it? I.e. 6 or 8. What is the >> use to request more then can be returned? > The application might not know or care exactly which size a MAC > address has on a specific device. > You are just making the application more complicated and I can't see > any benefits. > > -- Ola If we support case that application does not know, then ok. I will change it to addr_size < ETH_ALEN and add ODP_ERR there. Maxim. >> Maxim. >> >>>> I did linux-generic implementation for hw which I have. EUI-64 can be >>>> added >>>> with other >>>> patch which we should test first. Can EUI-64 be set up with the same >>>> ioctl >>>> or something else >>>> is needed? I need to take a look at it separately. >>>> >>>> Maxim. >>>> >>>> >>>>>> + >>>>>> + entry = get_entry(id); >>>>>> + if (entry == NULL) { >>>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>>> + return -ENOENT; >>>>>> + } >>>>>> + >>>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>>> + else >>>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>>> + >>>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>>> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >>>>>> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >>>>>> + >>>>>> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >>>>>> + if (ret < 0) { >>>>>> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >>>>>> + size_t addr_size) >>>>>> +{ >>>>>> + pktio_entry_t *entry; >>>>>> + int sockfd; >>>>>> + struct ifreq ifr; >>>>>> + int ret; >>>>>> + >>>>>> + if (addr_size < ETH_ALEN) >>>>>> + return -ENOMEM; >>>>>> + >>>>>> + entry = get_entry(id); >>>>>> + if (entry == NULL) { >>>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>>> + return -ENOENT; >>>>>> + } >>>>>> + >>>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>>> + else >>>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>>> + >>>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>>> + >>>>>> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >>>>>> + if (ret < 0) { >>>>>> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + memcpy(mac_addr, (unsigned char >>>>>> *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >>>>>> + ETH_ALEN); >>>>>> + >>>>>> + return ETH_ALEN; >>>>>> +} >>>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>
On 2 December 2014 at 23:18, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/03/2014 12:42 AM, Ola Liljedahl wrote: >> >> On 2 December 2014 at 21:55, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>> >>> On 12/02/2014 11:44 PM, Ola Liljedahl wrote: >>>> >>>> On 2 December 2014 at 21:37, Maxim Uvarov <maxim.uvarov@linaro.org> >>>> wrote: >>>>> >>>>> On 12/02/2014 08:11 PM, Zoltan Kiss wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 01/12/14 16:15, Maxim Uvarov wrote: >>>>>>> >>>>>>> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >>>>>>> else >>>>>>> return 0; >>>>>>> } >>>>>>> + >>>>>>> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >>>>>>> *mac_addr, >>>>>>> + size_t addr_size) >>>>>>> +{ >>>>>>> + pktio_entry_t *entry; >>>>>>> + int sockfd; >>>>>>> + struct ifreq ifr; >>>>>>> + int ret; >>>>>>> + >>>>>>> + if (addr_size != ETH_ALEN) >>>>>>> + return -ENOMEM; >>>>>> >>>>>> What about EUI-64 MAC addresses? Do we need/want to care about them? >>>> >>>> Yes why such a restrictive check? >>>> Why not just "if (addr_size < ETH_ALEN)" above? >>>> I can't see why it should be an error to provide a buffer that is >>>> larger than absolutely necessary. >>>> >>>> -- Ola >>> >>> >>> Application should request valid size, isn't it? I.e. 6 or 8. What is the >>> use to request more then can be returned? >> >> The application might not know or care exactly which size a MAC >> address has on a specific device. >> You are just making the application more complicated and I can't see >> any benefits. >> >> -- Ola > > If we support case that application does not know, then ok. I will change it > to addr_size < ETH_ALEN > and add ODP_ERR there. Not sure about the ODP_ERR. Doesn't the ODP architecture specify errno = something; return -1;? -- Ola > > Maxim. > > >>> Maxim. >>> >>>>> I did linux-generic implementation for hw which I have. EUI-64 can be >>>>> added >>>>> with other >>>>> patch which we should test first. Can EUI-64 be set up with the same >>>>> ioctl >>>>> or something else >>>>> is needed? I need to take a look at it separately. >>>>> >>>>> Maxim. >>>>> >>>>> >>>>>>> + >>>>>>> + entry = get_entry(id); >>>>>>> + if (entry == NULL) { >>>>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>>>> + return -ENOENT; >>>>>>> + } >>>>>>> + >>>>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>>>> + else >>>>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>>>> + >>>>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>>>> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >>>>>>> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >>>>>>> + >>>>>>> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >>>>>>> + if (ret < 0) { >>>>>>> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >>>>>>> + return ret; >>>>>>> + } >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >>>>>>> + size_t addr_size) >>>>>>> +{ >>>>>>> + pktio_entry_t *entry; >>>>>>> + int sockfd; >>>>>>> + struct ifreq ifr; >>>>>>> + int ret; >>>>>>> + >>>>>>> + if (addr_size < ETH_ALEN) >>>>>>> + return -ENOMEM; >>>>>>> + >>>>>>> + entry = get_entry(id); >>>>>>> + if (entry == NULL) { >>>>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>>>> + return -ENOENT; >>>>>>> + } >>>>>>> + >>>>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>>>> + else >>>>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>>>> + >>>>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>>>> + >>>>>>> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >>>>>>> + if (ret < 0) { >>>>>>> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >>>>>>> + return ret; >>>>>>> + } >>>>>>> + >>>>>>> + memcpy(mac_addr, (unsigned char >>>>>>> *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >>>>>>> + ETH_ALEN); >>>>>>> + >>>>>>> + return ETH_ALEN; >>>>>>> +} >>>>>>> >>>>>> _______________________________________________ >>>>>> lng-odp mailing list >>>>>> lng-odp@lists.linaro.org >>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >
On 12/03/2014 01:31 AM, Ola Liljedahl wrote: > On 2 December 2014 at 23:18, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> On 12/03/2014 12:42 AM, Ola Liljedahl wrote: >>> On 2 December 2014 at 21:55, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>>> On 12/02/2014 11:44 PM, Ola Liljedahl wrote: >>>>> On 2 December 2014 at 21:37, Maxim Uvarov <maxim.uvarov@linaro.org> >>>>> wrote: >>>>>> On 12/02/2014 08:11 PM, Zoltan Kiss wrote: >>>>>>> >>>>>>> >>>>>>> On 01/12/14 16:15, Maxim Uvarov wrote: >>>>>>>> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >>>>>>>> else >>>>>>>> return 0; >>>>>>>> } >>>>>>>> + >>>>>>>> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >>>>>>>> *mac_addr, >>>>>>>> + size_t addr_size) >>>>>>>> +{ >>>>>>>> + pktio_entry_t *entry; >>>>>>>> + int sockfd; >>>>>>>> + struct ifreq ifr; >>>>>>>> + int ret; >>>>>>>> + >>>>>>>> + if (addr_size != ETH_ALEN) >>>>>>>> + return -ENOMEM; >>>>>>> What about EUI-64 MAC addresses? Do we need/want to care about them? >>>>> Yes why such a restrictive check? >>>>> Why not just "if (addr_size < ETH_ALEN)" above? >>>>> I can't see why it should be an error to provide a buffer that is >>>>> larger than absolutely necessary. >>>>> >>>>> -- Ola >>>> >>>> Application should request valid size, isn't it? I.e. 6 or 8. What is the >>>> use to request more then can be returned? >>> The application might not know or care exactly which size a MAC >>> address has on a specific device. >>> You are just making the application more complicated and I can't see >>> any benefits. >>> >>> -- Ola >> If we support case that application does not know, then ok. I will change it >> to addr_size < ETH_ALEN >> and add ODP_ERR there. > Not sure about the ODP_ERR. Doesn't the ODP architecture specify errno > = something; return -1;? > > -- Ola We spoke today about 40 minutes about return errors from functions and did not agreed. So plan is to stay just with -1 for function which returns error and 0 for function which passes. ODP_ERR does not call abort(). It just prints error. Maxim. >> Maxim. >> >> >>>> Maxim. >>>> >>>>>> I did linux-generic implementation for hw which I have. EUI-64 can be >>>>>> added >>>>>> with other >>>>>> patch which we should test first. Can EUI-64 be set up with the same >>>>>> ioctl >>>>>> or something else >>>>>> is needed? I need to take a look at it separately. >>>>>> >>>>>> Maxim. >>>>>> >>>>>> >>>>>>>> + >>>>>>>> + entry = get_entry(id); >>>>>>>> + if (entry == NULL) { >>>>>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>>>>> + return -ENOENT; >>>>>>>> + } >>>>>>>> + >>>>>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>>>>> + else >>>>>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>>>>> + >>>>>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>>>>> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >>>>>>>> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >>>>>>>> + >>>>>>>> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >>>>>>>> + if (ret < 0) { >>>>>>>> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >>>>>>>> + return ret; >>>>>>>> + } >>>>>>>> + >>>>>>>> + return 0; >>>>>>>> +} >>>>>>>> + >>>>>>>> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >>>>>>>> + size_t addr_size) >>>>>>>> +{ >>>>>>>> + pktio_entry_t *entry; >>>>>>>> + int sockfd; >>>>>>>> + struct ifreq ifr; >>>>>>>> + int ret; >>>>>>>> + >>>>>>>> + if (addr_size < ETH_ALEN) >>>>>>>> + return -ENOMEM; >>>>>>>> + >>>>>>>> + entry = get_entry(id); >>>>>>>> + if (entry == NULL) { >>>>>>>> + ODP_DBG("pktio entry %d does not exist\n", id); >>>>>>>> + return -ENOENT; >>>>>>>> + } >>>>>>>> + >>>>>>>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>>>>>>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>>>>>>> + else >>>>>>>> + sockfd = entry->s.pkt_sock.sockfd; >>>>>>>> + >>>>>>>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>>>>>>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>>>>>>> + >>>>>>>> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >>>>>>>> + if (ret < 0) { >>>>>>>> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >>>>>>>> + return ret; >>>>>>>> + } >>>>>>>> + >>>>>>>> + memcpy(mac_addr, (unsigned char >>>>>>>> *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >>>>>>>> + ETH_ALEN); >>>>>>>> + >>>>>>>> + return ETH_ALEN; >>>>>>>> +} >>>>>>>> >>>>>>> _______________________________________________ >>>>>>> lng-odp mailing list >>>>>>> lng-odp@lists.linaro.org >>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> lng-odp mailing list >>>>>> lng-odp@lists.linaro.org >>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>
On 02/12/14 20:37, Maxim Uvarov wrote: > On 12/02/2014 08:11 PM, Zoltan Kiss wrote: >> >> >> On 01/12/14 16:15, Maxim Uvarov wrote: >>> @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) >>> else >>> return 0; >>> } >>> + >>> +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char >>> *mac_addr, >>> + size_t addr_size) >>> +{ >>> + pktio_entry_t *entry; >>> + int sockfd; >>> + struct ifreq ifr; >>> + int ret; >>> + >>> + if (addr_size != ETH_ALEN) >>> + return -ENOMEM; >> What about EUI-64 MAC addresses? Do we need/want to care about them? > > I did linux-generic implementation for hw which I have. EUI-64 can be > added with other > patch which we should test first. Can EUI-64 be set up with the same > ioctl or something else > is needed? I need to take a look at it separately. Another related thought: the ioctl takes a struct sockaddr, so the provided buffer shouldn't be bigger than 14 bytes. Also, sa_type should match the device's type. I don't know whether we should care about e.g. Fibre Channel, but if yes, then we need to add an additional parameter for these functions. But I guess we are fine with Ethernet right now. > > Maxim. > >> >>> + >>> + entry = get_entry(id); >>> + if (entry == NULL) { >>> + ODP_DBG("pktio entry %d does not exist\n", id); >>> + return -ENOENT; >>> + } >>> + >>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>> + else >>> + sockfd = entry->s.pkt_sock.sockfd; >>> + >>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>> + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); >>> + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; >>> + >>> + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); >>> + if (ret < 0) { >>> + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, >>> + size_t addr_size) >>> +{ >>> + pktio_entry_t *entry; >>> + int sockfd; >>> + struct ifreq ifr; >>> + int ret; >>> + >>> + if (addr_size < ETH_ALEN) >>> + return -ENOMEM; >>> + >>> + entry = get_entry(id); >>> + if (entry == NULL) { >>> + ODP_DBG("pktio entry %d does not exist\n", id); >>> + return -ENOENT; >>> + } >>> + >>> + if (entry->s.pkt_sock_mmap.sockfd > -1) >>> + sockfd = entry->s.pkt_sock_mmap.sockfd; >>> + else >>> + sockfd = entry->s.pkt_sock.sockfd; >>> + >>> + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); >>> + ifr.ifr_name[IFNAMSIZ - 1] = 0; >>> + >>> + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); >>> + if (ret < 0) { >>> + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); >>> + return ret; >>> + } >>> + >>> + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, >>> + ETH_ALEN); >>> + >>> + return ETH_ALEN; >>> +} >>> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h index c7d1e40..65adbe5 100644 --- a/platform/linux-generic/include/api/odp_packet_io.h +++ b/platform/linux-generic/include/api/odp_packet_io.h @@ -173,6 +173,31 @@ int odp_pktio_promisc_set(odp_pktio_t id, odp_bool_t enable); int odp_pktio_promisc_enabled(odp_pktio_t id); /** + * Set the default MAC address of a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[in] mac_addr MAC address to be assigned to the interface. + * @param[in] addr_size Size of the address in bytes. + * + * @return 0 on success, -ERROR on error. + */ +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char *mac_addr, + size_t addr_size); + +/** + * Get the default MAC address of a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[out] mac_addr Storage for MAC address of the packet IO interface. + * @param[in] addr_size Storage size for the address + * + * @retval Address size written. + * @retval -ERROR on error. + */ +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, + size_t addr_size); + +/** * @} */ diff --git a/platform/linux-generic/include/api/odp_std_types.h b/platform/linux-generic/include/api/odp_std_types.h index 61255a6..01c0377 100644 --- a/platform/linux-generic/include/api/odp_std_types.h +++ b/platform/linux-generic/include/api/odp_std_types.h @@ -26,6 +26,8 @@ extern "C" { #include <stdint.h> #include <inttypes.h> #include <limits.h> +#include <unistd.h> +#include <errno.h> /** Use odp boolean type to have it well-defined and known size, * regardless which compiler is used as this facilities interoperability diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index d97910f..d9c2425 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -21,6 +21,7 @@ #include <string.h> #include <sys/ioctl.h> +#include <linux/if_arp.h> typedef struct { pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; @@ -619,3 +620,76 @@ int odp_pktio_promisc_enabled(odp_pktio_t id) else return 0; } + +int odp_pktio_mac_addr_set(odp_pktio_t id, const unsigned char *mac_addr, + size_t addr_size) +{ + pktio_entry_t *entry; + int sockfd; + struct ifreq ifr; + int ret; + + if (addr_size != ETH_ALEN) + return -ENOMEM; + + entry = get_entry(id); + if (entry == NULL) { + ODP_DBG("pktio entry %d does not exist\n", id); + return -ENOENT; + } + + if (entry->s.pkt_sock_mmap.sockfd > -1) + sockfd = entry->s.pkt_sock_mmap.sockfd; + else + sockfd = entry->s.pkt_sock.sockfd; + + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); + ifr.ifr_name[IFNAMSIZ - 1] = 0; + memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, addr_size); + ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; + + ret = ioctl(sockfd, SIOCSIFHWADDR, &ifr); + if (ret < 0) { + ODP_DBG("ioctl SIOCSIFHWADDR error\n"); + return ret; + } + + return 0; +} + +ssize_t odp_pktio_mac_addr(odp_pktio_t id, unsigned char *mac_addr, + size_t addr_size) +{ + pktio_entry_t *entry; + int sockfd; + struct ifreq ifr; + int ret; + + if (addr_size < ETH_ALEN) + return -ENOMEM; + + entry = get_entry(id); + if (entry == NULL) { + ODP_DBG("pktio entry %d does not exist\n", id); + return -ENOENT; + } + + if (entry->s.pkt_sock_mmap.sockfd > -1) + sockfd = entry->s.pkt_sock_mmap.sockfd; + else + sockfd = entry->s.pkt_sock.sockfd; + + strncpy(ifr.ifr_name, entry->s.name, IFNAMSIZ - 1); + ifr.ifr_name[IFNAMSIZ - 1] = 0; + + ret = ioctl(sockfd, SIOCGIFHWADDR, &ifr); + if (ret < 0) { + ODP_DBG("ioctl SIOCGIFHWADDR error\n"); + return ret; + } + + memcpy(mac_addr, (unsigned char *)ifr.ifr_ifru.ifru_hwaddr.sa_data, + ETH_ALEN); + + return ETH_ALEN; +}
Define API for mac address change and implement linux-generic version. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/include/api/odp_packet_io.h | 25 ++++++++ platform/linux-generic/include/api/odp_std_types.h | 2 + platform/linux-generic/odp_packet_io.c | 74 ++++++++++++++++++++++ 3 files changed, 101 insertions(+)