On 09/08/2016 23:55, "Ilya Maximets" <i.maxim...@samsung.com> 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.maxim...@samsung.com>
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 <diproiet...@vmware.com>
>>
>> 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 <diproiet...@vmware.com>
>> ---
>> 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.ovsschema
>> index 32fdf28..8966803 100644
>> --- a/vswitchd/vswitch.ovsschema
>> +++ b/vswitchd/vswitch.ovsschema
>> @@ -1,6 +1,6 @@
>> {"name": "Open_vSwitch",
>> - "version": "7.13.0",
>> - "cksum": "889248633 22774",
>> + "version": "7.14.0",
>> + "cksum": "3974332717 22936",
>> "tables": {
>> "Open_vSwitch": {
>> "columns": {
>> @@ -321,6 +321,12 @@
>> "mtu": {
>> "type": {"key": "integer", "min": 0, "max": 1},
>> "ephemeral": true},
>> + "mtu_request": {
>> + "type": {
>> + "key": {"type": "integer",
>> + "minInteger": 1},
>> + "min": 0,
>> + "max": 1}},
>> "error": {
>> "type": {"key": "string", "min": 0, "max": 1}}},
>> "indexes": [["name"]]},
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 65acdc7..780bd2d 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -2380,6 +2380,44 @@
>> </column>
>> </group>
>>
>> + <group title="MTU">
>> + <p>
>> + The MTU (maximum transmission unit) is the largest amount of data
>> + that can fit into a single Ethernet frame. The standard Ethernet
>> + MTU is 1500 bytes. Some physical media and many kinds of virtual
>> + interfaces can be configured with higher MTUs.
>> + </p>
>> +
>> + <p>
>> + A client may change a non-internal interface MTU by filling in
>> + <ref column="mtu_request"/>. Internal interfaces MTU, instead, is
>> set
>> + by Open vSwitch to the minimum of non-internal interfaces MTU in the
>> + bridge. In any case, Open vSwitch then reports in <ref
>> column="mtu"/>
>> + the currently configured value.
>> + </p>
>> +
>> + <column name="mtu">
>> + <p>
>> + This column will be empty for an interface that does not
>> + have an MTU as, for example, some kinds of tunnels do not.
>> + </p>
>> +
>> + <p>
>> + Open vSwitch sets this column's value, so other clients should
>> treat
>> + it as read-only.
>> + </p>
>> + </column>
>> +
>> + <column name="mtu_request"
>> + type='{"type": "integer", "minInteger": 1}'>
>> + <p>
>> + Requested MTU (Maximum Transmission Unit) for the interface. A
>> client
>> + can fill this column to change the MTU of a non-internal
>> interface.
>> + </p>
>> + </column>
>> +
>> + </group>
>> +
>> <group title="Interface Status">
>> <p>
>> Status information about interfaces attached to bridges, updated
>> every
>> @@ -2422,20 +2460,6 @@
>> </p>
>> </column>
>>
>> - <column name="mtu">
>> - <p>
>> - The MTU (maximum transmission unit); i.e. the largest
>> - amount of data that can fit into a single Ethernet frame.
>> - The standard Ethernet MTU is 1500 bytes. Some physical media
>> - and many kinds of virtual interfaces can be configured with
>> - higher MTUs.
>> - </p>
>> - <p>
>> - This column will be empty for an interface that does not
>> - have an MTU as, for example, some kinds of tunnels do not.
>> - </p>
>> - </column>
>> -
>> <column name="lacp_current">
>> Boolean value indicating LACP status for this interface. If true,
>> this
>> interface has current LACP information about its LACP partner. This
>>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev