Re: [ovs-dev] [PATCH V7 2/7] vswitchd: Introduce 'mtu_request' column in Interface.
Hi, Daniele.
Maybe I'm not the very right person to review such things, but I'm
actually used this patch since it appeared in your github and it
looks good to me. Also, I wanted to remove this non-working
implementation of 'netdev_dpdk_set_mtu' for a long time.
Acked-by: Ilya Maximets
On 09.08.2016 19:01, Mark Kavanagh wrote:
> From: Daniele Di Proietto
>
> The 'mtu_request' column can be used to set the MTU of a specific
> interface.
>
> This column is useful because it will allow changing the MTU of DPDK
> devices (implemented in a future commit), which are not accessible
> outside the ovs-vswitchd process, but it can be used for kernel
> interfaces as well.
>
> The current implementation of set_mtu() in netdev-dpdk is removed
> because it's broken. It will be reintroduced by a subsequent commit on
> this series.
>
> Signed-off-by: Daniele Di Proietto
> ---
> NEWS | 2 ++
> lib/netdev-dpdk.c | 53
> +-
> vswitchd/bridge.c | 9
> vswitchd/vswitch.ovsschema | 10 +++--
> vswitchd/vswitch.xml | 52 +
> 5 files changed, 58 insertions(+), 68 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index c2ed71d..ce10982 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -101,6 +101,8 @@ Post-v2.5.0
> - ovs-pki: Changed message digest algorithm from SHA-1 to SHA-512 because
> SHA-1 is no longer secure and some operating systems have started to
> disable it in OpenSSL.
> + - Add 'mtu_request' column to the Interface table. It can be used to
> + configure the MTU of non-internal ports.
>
>
> v2.5.0 - 26 Feb 2016
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index f37ec1c..60db568 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1639,57 +1639,6 @@ netdev_dpdk_get_mtu(const struct netdev *netdev, int
> *mtup)
> }
>
> static int
> -netdev_dpdk_set_mtu(const struct netdev *netdev, int mtu)
> -{
> -struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> -int old_mtu, err, dpdk_mtu;
> -struct dpdk_mp *old_mp;
> -struct dpdk_mp *mp;
> -uint32_t buf_size;
> -
> -ovs_mutex_lock(&dpdk_mutex);
> -ovs_mutex_lock(&dev->mutex);
> -if (dev->mtu == mtu) {
> -err = 0;
> -goto out;
> -}
> -
> -buf_size = dpdk_buf_size(mtu);
> -dpdk_mtu = FRAME_LEN_TO_MTU(buf_size);
> -
> -mp = dpdk_mp_get(dev->socket_id, dpdk_mtu);
> -if (!mp) {
> -err = ENOMEM;
> -goto out;
> -}
> -
> -rte_eth_dev_stop(dev->port_id);
> -
> -old_mtu = dev->mtu;
> -old_mp = dev->dpdk_mp;
> -dev->dpdk_mp = mp;
> -dev->mtu = mtu;
> -dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
> -
> -err = dpdk_eth_dev_init(dev);
> -if (err) {
> -dpdk_mp_put(mp);
> -dev->mtu = old_mtu;
> -dev->dpdk_mp = old_mp;
> -dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
> -dpdk_eth_dev_init(dev);
> -goto out;
> -}
> -
> -dpdk_mp_put(old_mp);
> -netdev_change_seq_changed(netdev);
> -out:
> -ovs_mutex_unlock(&dev->mutex);
> -ovs_mutex_unlock(&dpdk_mutex);
> -return err;
> -}
> -
> -static int
> netdev_dpdk_get_carrier(const struct netdev *netdev, bool *carrier);
>
> static int
> @@ -2964,7 +2913,7 @@ netdev_dpdk_vhost_cuse_reconfigure(struct netdev
> *netdev)
> netdev_dpdk_set_etheraddr,\
> netdev_dpdk_get_etheraddr,\
> netdev_dpdk_get_mtu, \
> -netdev_dpdk_set_mtu, \
> +NULL, /* set_mtu */ \
> netdev_dpdk_get_ifindex, \
> GET_CARRIER, \
> netdev_dpdk_get_carrier_resets, \
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index ddf1fe5..397be70 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -775,6 +775,15 @@ bridge_delete_or_reconfigure_ports(struct bridge *br)
> goto delete;
> }
>
> +if (iface->cfg->n_mtu_request == 1
> +&& strcmp(iface->type,
> + ofproto_port_open_type(br->type, "internal"))) {
> +/* Try to set the MTU to the requested value. This is not done
> + * for internal interfaces, since their MTU is decided by the
> + * ofproto module, based on other ports in the bridge. */
> +netdev_set_mtu(iface->netdev, *iface->cfg->mtu_request);
> +}
> +
> /* If the requested OpenFlow port for 'iface' changed, and it's not
> * already the correct port, then we might want to temporarily delete
> * this interface, so we can add it back again with the new OpenFlow
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovssc
Re: [ovs-dev] [PATCH V7 2/7] vswitchd: Introduce 'mtu_request' column in Interface.
On 09/08/2016 23:55, "Ilya Maximets" wrote:
>Hi, Daniele.
>Maybe I'm not the very right person to review such things, but I'm
>actually used this patch since it appeared in your github and it
>looks good to me. Also, I wanted to remove this non-working
>implementation of 'netdev_dpdk_set_mtu' for a long time.
>
>Acked-by: Ilya Maximets
I just wanted to give it a couple of more days to see if there were
any objections for the database interface.
Thanks for the review, I pushed this to master
>
>On 09.08.2016 19:01, Mark Kavanagh wrote:
>> From: Daniele Di Proietto
>>
>> The 'mtu_request' column can be used to set the MTU of a specific
>> interface.
>>
>> This column is useful because it will allow changing the MTU of DPDK
>> devices (implemented in a future commit), which are not accessible
>> outside the ovs-vswitchd process, but it can be used for kernel
>> interfaces as well.
>>
>> The current implementation of set_mtu() in netdev-dpdk is removed
>> because it's broken. It will be reintroduced by a subsequent commit on
>> this series.
>>
>> Signed-off-by: Daniele Di Proietto
>> ---
>> NEWS | 2 ++
>> lib/netdev-dpdk.c | 53
>> +-
>> vswitchd/bridge.c | 9
>> vswitchd/vswitch.ovsschema | 10 +++--
>> vswitchd/vswitch.xml | 52
>> +
>> 5 files changed, 58 insertions(+), 68 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index c2ed71d..ce10982 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -101,6 +101,8 @@ Post-v2.5.0
>> - ovs-pki: Changed message digest algorithm from SHA-1 to SHA-512 because
>> SHA-1 is no longer secure and some operating systems have started to
>> disable it in OpenSSL.
>> + - Add 'mtu_request' column to the Interface table. It can be used to
>> + configure the MTU of non-internal ports.
>>
>>
>> v2.5.0 - 26 Feb 2016
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index f37ec1c..60db568 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1639,57 +1639,6 @@ netdev_dpdk_get_mtu(const struct netdev *netdev, int
>> *mtup)
>> }
>>
>> static int
>> -netdev_dpdk_set_mtu(const struct netdev *netdev, int mtu)
>> -{
>> -struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>> -int old_mtu, err, dpdk_mtu;
>> -struct dpdk_mp *old_mp;
>> -struct dpdk_mp *mp;
>> -uint32_t buf_size;
>> -
>> -ovs_mutex_lock(&dpdk_mutex);
>> -ovs_mutex_lock(&dev->mutex);
>> -if (dev->mtu == mtu) {
>> -err = 0;
>> -goto out;
>> -}
>> -
>> -buf_size = dpdk_buf_size(mtu);
>> -dpdk_mtu = FRAME_LEN_TO_MTU(buf_size);
>> -
>> -mp = dpdk_mp_get(dev->socket_id, dpdk_mtu);
>> -if (!mp) {
>> -err = ENOMEM;
>> -goto out;
>> -}
>> -
>> -rte_eth_dev_stop(dev->port_id);
>> -
>> -old_mtu = dev->mtu;
>> -old_mp = dev->dpdk_mp;
>> -dev->dpdk_mp = mp;
>> -dev->mtu = mtu;
>> -dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
>> -
>> -err = dpdk_eth_dev_init(dev);
>> -if (err) {
>> -dpdk_mp_put(mp);
>> -dev->mtu = old_mtu;
>> -dev->dpdk_mp = old_mp;
>> -dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
>> -dpdk_eth_dev_init(dev);
>> -goto out;
>> -}
>> -
>> -dpdk_mp_put(old_mp);
>> -netdev_change_seq_changed(netdev);
>> -out:
>> -ovs_mutex_unlock(&dev->mutex);
>> -ovs_mutex_unlock(&dpdk_mutex);
>> -return err;
>> -}
>> -
>> -static int
>> netdev_dpdk_get_carrier(const struct netdev *netdev, bool *carrier);
>>
>> static int
>> @@ -2964,7 +2913,7 @@ netdev_dpdk_vhost_cuse_reconfigure(struct netdev
>> *netdev)
>> netdev_dpdk_set_etheraddr,\
>> netdev_dpdk_get_etheraddr,\
>> netdev_dpdk_get_mtu, \
>> -netdev_dpdk_set_mtu, \
>> +NULL, /* set_mtu */ \
>> netdev_dpdk_get_ifindex, \
>> GET_CARRIER, \
>> netdev_dpdk_get_carrier_resets, \
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index ddf1fe5..397be70 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -775,6 +775,15 @@ bridge_delete_or_reconfigure_ports(struct bridge *br)
>> goto delete;
>> }
>>
>> +if (iface->cfg->n_mtu_request == 1
>> +&& strcmp(iface->type,
>> + ofproto_port_open_type(br->type, "internal"))) {
>> +/* Try to set the MTU to the requested value. This is not done
>> + * for internal interfaces, since their MTU is decided by the
>> + * ofproto module, based on other ports in the bridge. */
>> +netdev_set_mtu(iface->netdev,
