[dpdk-dev,v2] net/null:Different mac address support
Checks
Commit Message
After attaching two Null device to ovs, seeing "00.00.00.00.00.00" mac
address for both null devices. Fix this issue, by setting different mac
address.
Signed-off-by: Mallesh Koujalagi <malleshx.koujalagi@intel.com>
---
drivers/net/null/rte_eth_null.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
Comments
On Mon, 5 Mar 2018 19:35:14 -0800
Mallesh Koujalagi <malleshx.koujalagi@intel.com> wrote:
> After attaching two Null device to ovs, seeing "00.00.00.00.00.00" mac
> address for both null devices. Fix this issue, by setting different mac
> address.
>
> Signed-off-by: Mallesh Koujalagi <malleshx.koujalagi@intel.com>
> ---
> drivers/net/null/rte_eth_null.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> index 9385ffd..599b513 100644
> --- a/drivers/net/null/rte_eth_null.c
> +++ b/drivers/net/null/rte_eth_null.c
> @@ -85,8 +85,17 @@ struct pmd_internals {
> uint8_t rss_key[40]; /**< 40-byte hash key. */
> };
>
> +static struct ether_addr base_eth_addr = {
> + .addr_bytes = {
> + 0x4E /* N */,
> + 0x55 /* U */,
> + 0x4C /* L */,
> + 0x4C /* L */,
> + 0x00,
> + 0x00
> + }
> +};
Cute, but since first octets of Ethernet are the vendor id (OUI)
it might be confusing. At least 'N' is 4E which does not have
the group address (multicast) set; and it does have the local
admin address bit set.
You really should be using a random locally assigned value.
> -static struct ether_addr eth_addr = { .addr_bytes = {0} };
> static struct rte_eth_link pmd_link = {
> .link_speed = ETH_SPEED_NUM_10G,
> .link_duplex = ETH_LINK_FULL_DUPLEX,
> @@ -492,6 +501,7 @@ eth_dev_null_create(struct rte_vdev_device *dev,
> struct rte_eth_dev_data *data = NULL;
> struct pmd_internals *internals = NULL;
> struct rte_eth_dev *eth_dev = NULL;
> + struct ether_addr *eth_addr = NULL;
>
> static const uint8_t default_rss_key[40] = {
> 0x6D, 0x5A, 0x56, 0xDA, 0x25, 0x5B, 0x0E, 0xC2, 0x41, 0x67, 0x25, 0x3D,
> @@ -514,12 +524,21 @@ eth_dev_null_create(struct rte_vdev_device *dev,
> if (!data)
> return -ENOMEM;
>
> + eth_addr = rte_zmalloc_socket(rte_vdev_device_name(dev),
> + sizeof(*eth_addr), 0, dev->device.numa_node);
> + if (eth_addr == NULL) {
> + rte_free(data);
> + return -ENOMEM;
> + }
> +
> eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internals));
> if (!eth_dev) {
> + rte_free(eth_addr);
> rte_free(data);
> return -ENOMEM;
> }
> -
> + *eth_addr = base_eth_addr;
> + eth_addr->addr_bytes[5] = eth_dev->data->port_id;
> /* now put it all together
> * - store queue data in internals,
> * - store numa_node info in ethdev data
> @@ -543,7 +562,7 @@ eth_dev_null_create(struct rte_vdev_device *dev,
> data->nb_rx_queues = (uint16_t)nb_rx_queues;
> data->nb_tx_queues = (uint16_t)nb_tx_queues;
> data->dev_link = pmd_link;
> - data->mac_addrs = ð_addr;
> + data->mac_addrs = eth_addr;
>
> eth_dev->data = data;
> eth_dev->dev_ops = &ops;
> @@ -662,6 +681,7 @@ rte_pmd_null_remove(struct rte_vdev_device *dev)
> if (eth_dev == NULL)
> return -1;
>
> + rte_free(eth_dev->data->mac_addrs);
> rte_free(eth_dev->data->dev_private);
> rte_free(eth_dev->data);
>
On 3/6/2018 3:35 AM, Mallesh Koujalagi wrote:
> After attaching two Null device to ovs, seeing "00.00.00.00.00.00" mac
> address for both null devices. Fix this issue, by setting different mac
> address.
>
> Signed-off-by: Mallesh Koujalagi <malleshx.koujalagi@intel.com>
<...>
> @@ -514,12 +524,21 @@ eth_dev_null_create(struct rte_vdev_device *dev,
> if (!data)
> return -ENOMEM;
>
> + eth_addr = rte_zmalloc_socket(rte_vdev_device_name(dev),
> + sizeof(*eth_addr), 0, dev->device.numa_node);
> + if (eth_addr == NULL) {
> + rte_free(data);
> + return -ENOMEM;
> + }
> +
> eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internals));
> if (!eth_dev) {
> + rte_free(eth_addr);
> rte_free(data);
> return -ENOMEM;
> }
Same comment from previous version, why not put "eth_addr" inside "struct
pmd_internals"?
"struct pmd_internals" is already allocated/freed in the code, so you don't need
to manage "eth_addr" if you put it into "struct pmd_internals" it will come free.
<...>
@@ -85,8 +85,17 @@ struct pmd_internals {
uint8_t rss_key[40]; /**< 40-byte hash key. */
};
+static struct ether_addr base_eth_addr = {
+ .addr_bytes = {
+ 0x4E /* N */,
+ 0x55 /* U */,
+ 0x4C /* L */,
+ 0x4C /* L */,
+ 0x00,
+ 0x00
+ }
+};
-static struct ether_addr eth_addr = { .addr_bytes = {0} };
static struct rte_eth_link pmd_link = {
.link_speed = ETH_SPEED_NUM_10G,
.link_duplex = ETH_LINK_FULL_DUPLEX,
@@ -492,6 +501,7 @@ eth_dev_null_create(struct rte_vdev_device *dev,
struct rte_eth_dev_data *data = NULL;
struct pmd_internals *internals = NULL;
struct rte_eth_dev *eth_dev = NULL;
+ struct ether_addr *eth_addr = NULL;
static const uint8_t default_rss_key[40] = {
0x6D, 0x5A, 0x56, 0xDA, 0x25, 0x5B, 0x0E, 0xC2, 0x41, 0x67, 0x25, 0x3D,
@@ -514,12 +524,21 @@ eth_dev_null_create(struct rte_vdev_device *dev,
if (!data)
return -ENOMEM;
+ eth_addr = rte_zmalloc_socket(rte_vdev_device_name(dev),
+ sizeof(*eth_addr), 0, dev->device.numa_node);
+ if (eth_addr == NULL) {
+ rte_free(data);
+ return -ENOMEM;
+ }
+
eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internals));
if (!eth_dev) {
+ rte_free(eth_addr);
rte_free(data);
return -ENOMEM;
}
-
+ *eth_addr = base_eth_addr;
+ eth_addr->addr_bytes[5] = eth_dev->data->port_id;
/* now put it all together
* - store queue data in internals,
* - store numa_node info in ethdev data
@@ -543,7 +562,7 @@ eth_dev_null_create(struct rte_vdev_device *dev,
data->nb_rx_queues = (uint16_t)nb_rx_queues;
data->nb_tx_queues = (uint16_t)nb_tx_queues;
data->dev_link = pmd_link;
- data->mac_addrs = ð_addr;
+ data->mac_addrs = eth_addr;
eth_dev->data = data;
eth_dev->dev_ops = &ops;
@@ -662,6 +681,7 @@ rte_pmd_null_remove(struct rte_vdev_device *dev)
if (eth_dev == NULL)
return -1;
+ rte_free(eth_dev->data->mac_addrs);
rte_free(eth_dev->data->dev_private);
rte_free(eth_dev->data);