* [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
@ 2013-08-13 17:55 Vlad Yasevich
2013-08-13 18:05 ` Sergei Shtylyov
2013-08-14 12:10 ` Michael S. Tsirkin
0 siblings, 2 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-08-13 17:55 UTC (permalink / raw)
To: netdev; +Cc: mst, Vlad Yasevich
When the user turns off IFF_VNET_HDR flag, attempts to change
offload features via TUNSETOFFLOAD do not work. This could cause
GSO packets to be delivered to the user when the user is
not prepared to handle them.
To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
disabled and make sure to turn off all offloads in this case.
Also, when IFF_VNET_HDR is disabled, run throught the offload change
as well to make sure that the functionality is not dependent on
the order of the ioclt() calls.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
drivers/net/macvtap.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a98fb0e..076b9e7 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
struct macvlan_dev *vlan;
netdev_features_t features;
netdev_features_t feature_mask = 0;
+ netdev_features_t tap_mask = TUN_OFFLOADS;
vlan = rtnl_dereference(q->vlan);
if (!vlan)
@@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
features = vlan->dev->features;
- if (arg & TUN_F_CSUM) {
+ if (!(q->flags & IFF_VNET_HDR)) {
+ /* Turn off all checsum offloading also if user does
+ * not user vnet_hdr.
+ */
+ tap_mask |= NETIF_F_ALL_CSUM;
+ } else if (arg & TUN_F_CSUM) {
feature_mask = NETIF_F_HW_CSUM;
if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
@@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
/* tap_features are the same as features on tun/tap and
* reflect user expectations.
*/
- vlan->tap_features = vlan->dev->features &
- (feature_mask | ~TUN_OFFLOADS);
+ vlan->tap_features = vlan->dev->features & (feature_mask | ~tap_mask);
vlan->set_features = features;
netdev_update_features(vlan->dev);
@@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
(IFF_NO_PI | IFF_TAP))
ret = -EINVAL;
- else
+ else {
+ if ((q->flags ^ u) & IFF_VNET_HDR) {
+ /* vnet_hdr support impacts the offloads,
+ * so we need to run throught the offload
+ * change.
+ */
+ rtnl_lock();
+ ret = set_offload(q, 0);
+ rtnl_unlock();
+ }
q->flags = u;
+ }
return ret;
@@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
TUN_F_TSO_ECN | TUN_F_UFO))
return -EINVAL;
- /* TODO: only accept frames with the features that
- got enabled for forwarded frames */
- if (!(q->flags & IFF_VNET_HDR))
- return -EINVAL;
rtnl_lock();
ret = set_offload(q, arg);
rtnl_unlock();
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
2013-08-13 17:55 [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled Vlad Yasevich
@ 2013-08-13 18:05 ` Sergei Shtylyov
2013-08-14 12:10 ` Michael S. Tsirkin
1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2013-08-13 18:05 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, mst
Hello.
On 08/13/2013 09:55 PM, Vlad Yasevich wrote:
> When the user turns off IFF_VNET_HDR flag, attempts to change
> offload features via TUNSETOFFLOAD do not work. This could cause
> GSO packets to be delivered to the user when the user is
> not prepared to handle them.
> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> disabled and make sure to turn off all offloads in this case.
> Also, when IFF_VNET_HDR is disabled, run throught the offload change
> as well to make sure that the functionality is not dependent on
> the order of the ioclt() calls.
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> drivers/net/macvtap.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index a98fb0e..076b9e7 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
[...]
> @@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
> (IFF_NO_PI | IFF_TAP))
> ret = -EINVAL;
> - else
> + else {
> + if ((q->flags ^ u) & IFF_VNET_HDR) {
> + /* vnet_hdr support impacts the offloads,
> + * so we need to run throught the offload
> + * change.
> + */
> + rtnl_lock();
> + ret = set_offload(q, 0);
> + rtnl_unlock();
> + }
> q->flags = u;
> + }
According to Documentation/CodingStyle, both arms of the *if* statement
should have {} if one arm has it.
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
2013-08-13 17:55 [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled Vlad Yasevich
2013-08-13 18:05 ` Sergei Shtylyov
@ 2013-08-14 12:10 ` Michael S. Tsirkin
2013-08-14 14:07 ` Vlad Yasevich
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-08-14 12:10 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
On Tue, Aug 13, 2013 at 01:55:58PM -0400, Vlad Yasevich wrote:
> When the user turns off IFF_VNET_HDR flag, attempts to change
> offload features via TUNSETOFFLOAD do not work. This could cause
> GSO packets to be delivered to the user when the user is
> not prepared to handle them.
>
> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> disabled and make sure to turn off all offloads in this case.
> Also, when IFF_VNET_HDR is disabled, run throught the offload change
> as well to make sure that the functionality is not dependent on
> the order of the ioclt() calls.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
Hmm, this seems to make it even messier.
So
set TUNSETOFFLOAD
clear IFF_VNET_HDR
set IFF_VNET_HDR
offloads are now clear?
I'm also not sure userspace that sets offloads but clears
vnet hdr knows what it's doing.
How about we fail attempts to clear vnet hdr
unless offloads are disabled?
> ---
> drivers/net/macvtap.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index a98fb0e..076b9e7 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> struct macvlan_dev *vlan;
> netdev_features_t features;
> netdev_features_t feature_mask = 0;
> + netdev_features_t tap_mask = TUN_OFFLOADS;
>
> vlan = rtnl_dereference(q->vlan);
> if (!vlan)
> @@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
>
> features = vlan->dev->features;
>
> - if (arg & TUN_F_CSUM) {
> + if (!(q->flags & IFF_VNET_HDR)) {
> + /* Turn off all checsum offloading also if user does
> + * not user vnet_hdr.
Does not set?
> + */
> + tap_mask |= NETIF_F_ALL_CSUM;
> + } else if (arg & TUN_F_CSUM) {
> feature_mask = NETIF_F_HW_CSUM;
>
> if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
> @@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> /* tap_features are the same as features on tun/tap and
> * reflect user expectations.
> */
> - vlan->tap_features = vlan->dev->features &
> - (feature_mask | ~TUN_OFFLOADS);
> + vlan->tap_features = vlan->dev->features & (feature_mask | ~tap_mask);
> vlan->set_features = features;
> netdev_update_features(vlan->dev);
>
> @@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
> (IFF_NO_PI | IFF_TAP))
> ret = -EINVAL;
> - else
> + else {
> + if ((q->flags ^ u) & IFF_VNET_HDR) {
> + /* vnet_hdr support impacts the offloads,
> + * so we need to run throught the offload
throught ?
> + * change.
> + */
> + rtnl_lock();
> + ret = set_offload(q, 0);
> + rtnl_unlock();
> + }
> q->flags = u;
> + }
>
> return ret;
>
> @@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> TUN_F_TSO_ECN | TUN_F_UFO))
> return -EINVAL;
>
> - /* TODO: only accept frames with the features that
> - got enabled for forwarded frames */
> - if (!(q->flags & IFF_VNET_HDR))
> - return -EINVAL;
> rtnl_lock();
> ret = set_offload(q, arg);
> rtnl_unlock();
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
2013-08-14 12:10 ` Michael S. Tsirkin
@ 2013-08-14 14:07 ` Vlad Yasevich
2013-08-14 16:04 ` Vlad Yasevich
0 siblings, 1 reply; 6+ messages in thread
From: Vlad Yasevich @ 2013-08-14 14:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
On 08/14/2013 08:10 AM, Michael S. Tsirkin wrote:
> On Tue, Aug 13, 2013 at 01:55:58PM -0400, Vlad Yasevich wrote:
>> When the user turns off IFF_VNET_HDR flag, attempts to change
>> offload features via TUNSETOFFLOAD do not work. This could cause
>> GSO packets to be delivered to the user when the user is
>> not prepared to handle them.
>>
>> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
>> disabled and make sure to turn off all offloads in this case.
>> Also, when IFF_VNET_HDR is disabled, run throught the offload change
>> as well to make sure that the functionality is not dependent on
>> the order of the ioclt() calls.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>
> Hmm, this seems to make it even messier.
> So
> set TUNSETOFFLOAD
> clear IFF_VNET_HDR
> set IFF_VNET_HDR
>
> offloads are now clear?
>
> I'm also not sure userspace that sets offloads but clears
> vnet hdr knows what it's doing.
>
> How about we fail attempts to clear vnet hdr
> unless offloads are disabled?
>
OK. That makes it simpler...
-vlad
>
>
>
>> ---
>> drivers/net/macvtap.c | 27 +++++++++++++++++++--------
>> 1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>> index a98fb0e..076b9e7 100644
>> --- a/drivers/net/macvtap.c
>> +++ b/drivers/net/macvtap.c
>> @@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
>> struct macvlan_dev *vlan;
>> netdev_features_t features;
>> netdev_features_t feature_mask = 0;
>> + netdev_features_t tap_mask = TUN_OFFLOADS;
>>
>> vlan = rtnl_dereference(q->vlan);
>> if (!vlan)
>> @@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
>>
>> features = vlan->dev->features;
>>
>> - if (arg & TUN_F_CSUM) {
>> + if (!(q->flags & IFF_VNET_HDR)) {
>> + /* Turn off all checsum offloading also if user does
>> + * not user vnet_hdr.
>
> Does not set?
>
>> + */
>> + tap_mask |= NETIF_F_ALL_CSUM;
>> + } else if (arg & TUN_F_CSUM) {
>> feature_mask = NETIF_F_HW_CSUM;
>>
>> if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
>> @@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
>> /* tap_features are the same as features on tun/tap and
>> * reflect user expectations.
>> */
>> - vlan->tap_features = vlan->dev->features &
>> - (feature_mask | ~TUN_OFFLOADS);
>> + vlan->tap_features = vlan->dev->features & (feature_mask | ~tap_mask);
>> vlan->set_features = features;
>> netdev_update_features(vlan->dev);
>>
>> @@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
>> if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
>> (IFF_NO_PI | IFF_TAP))
>> ret = -EINVAL;
>> - else
>> + else {
>> + if ((q->flags ^ u) & IFF_VNET_HDR) {
>> + /* vnet_hdr support impacts the offloads,
>> + * so we need to run throught the offload
>
> throught ?
>
>> + * change.
>> + */
>> + rtnl_lock();
>> + ret = set_offload(q, 0);
>> + rtnl_unlock();
>> + }
>> q->flags = u;
>> + }
>>
>> return ret;
>>
>> @@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
>> TUN_F_TSO_ECN | TUN_F_UFO))
>> return -EINVAL;
>>
>> - /* TODO: only accept frames with the features that
>> - got enabled for forwarded frames */
>> - if (!(q->flags & IFF_VNET_HDR))
>> - return -EINVAL;
>> rtnl_lock();
>> ret = set_offload(q, arg);
>> rtnl_unlock();
>> --
>> 1.8.1.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
2013-08-14 14:07 ` Vlad Yasevich
@ 2013-08-14 16:04 ` Vlad Yasevich
2013-08-14 17:43 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Vlad Yasevich @ 2013-08-14 16:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
On 08/14/2013 10:07 AM, Vlad Yasevich wrote:
> On 08/14/2013 08:10 AM, Michael S. Tsirkin wrote:
>> On Tue, Aug 13, 2013 at 01:55:58PM -0400, Vlad Yasevich wrote:
>>> When the user turns off IFF_VNET_HDR flag, attempts to change
>>> offload features via TUNSETOFFLOAD do not work. This could cause
>>> GSO packets to be delivered to the user when the user is
>>> not prepared to handle them.
>>>
>>> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
>>> disabled and make sure to turn off all offloads in this case.
>>> Also, when IFF_VNET_HDR is disabled, run throught the offload change
>>> as well to make sure that the functionality is not dependent on
>>> the order of the ioclt() calls.
>>>
>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>
>> Hmm, this seems to make it even messier.
>> So
>> set TUNSETOFFLOAD
>> clear IFF_VNET_HDR
>> set IFF_VNET_HDR
>>
>> offloads are now clear?
>>
>> I'm also not sure userspace that sets offloads but clears
>> vnet hdr knows what it's doing.
>>
>> How about we fail attempts to clear vnet hdr
>> unless offloads are disabled?
>>
>
> OK. That makes it simpler...
Spoke too soon.
What happens is that libvirt is typically the one that turns off
IFF_VNET_HDR, and qemu controls the offloads.
So, if we forbid turning off vnet_hdr while offloads are set, that
will break libvirt (this is because macvtap defaults to tap offloads
being enabled).
-vlad
>
> -vlad
>
>>
>>
>>
>>> ---
>>> drivers/net/macvtap.c | 27 +++++++++++++++++++--------
>>> 1 file changed, 19 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>>> index a98fb0e..076b9e7 100644
>>> --- a/drivers/net/macvtap.c
>>> +++ b/drivers/net/macvtap.c
>>> @@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q,
>>> unsigned long arg)
>>> struct macvlan_dev *vlan;
>>> netdev_features_t features;
>>> netdev_features_t feature_mask = 0;
>>> + netdev_features_t tap_mask = TUN_OFFLOADS;
>>>
>>> vlan = rtnl_dereference(q->vlan);
>>> if (!vlan)
>>> @@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue
>>> *q, unsigned long arg)
>>>
>>> features = vlan->dev->features;
>>>
>>> - if (arg & TUN_F_CSUM) {
>>> + if (!(q->flags & IFF_VNET_HDR)) {
>>> + /* Turn off all checsum offloading also if user does
>>> + * not user vnet_hdr.
>>
>> Does not set?
>>
>>> + */
>>> + tap_mask |= NETIF_F_ALL_CSUM;
>>> + } else if (arg & TUN_F_CSUM) {
>>> feature_mask = NETIF_F_HW_CSUM;
>>>
>>> if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
>>> @@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q,
>>> unsigned long arg)
>>> /* tap_features are the same as features on tun/tap and
>>> * reflect user expectations.
>>> */
>>> - vlan->tap_features = vlan->dev->features &
>>> - (feature_mask | ~TUN_OFFLOADS);
>>> + vlan->tap_features = vlan->dev->features & (feature_mask |
>>> ~tap_mask);
>>> vlan->set_features = features;
>>> netdev_update_features(vlan->dev);
>>>
>>> @@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file,
>>> unsigned int cmd,
>>> if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
>>> (IFF_NO_PI | IFF_TAP))
>>> ret = -EINVAL;
>>> - else
>>> + else {
>>> + if ((q->flags ^ u) & IFF_VNET_HDR) {
>>> + /* vnet_hdr support impacts the offloads,
>>> + * so we need to run throught the offload
>>
>> throught ?
>>
>>> + * change.
>>> + */
>>> + rtnl_lock();
>>> + ret = set_offload(q, 0);
>>> + rtnl_unlock();
>>> + }
>>> q->flags = u;
>>> + }
>>>
>>> return ret;
>>>
>>> @@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file,
>>> unsigned int cmd,
>>> TUN_F_TSO_ECN | TUN_F_UFO))
>>> return -EINVAL;
>>>
>>> - /* TODO: only accept frames with the features that
>>> - got enabled for forwarded frames */
>>> - if (!(q->flags & IFF_VNET_HDR))
>>> - return -EINVAL;
>>> rtnl_lock();
>>> ret = set_offload(q, arg);
>>> rtnl_unlock();
>>> --
>>> 1.8.1.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
2013-08-14 16:04 ` Vlad Yasevich
@ 2013-08-14 17:43 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-08-14 17:43 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
On Wed, Aug 14, 2013 at 12:04:22PM -0400, Vlad Yasevich wrote:
> On 08/14/2013 10:07 AM, Vlad Yasevich wrote:
> >On 08/14/2013 08:10 AM, Michael S. Tsirkin wrote:
> >>On Tue, Aug 13, 2013 at 01:55:58PM -0400, Vlad Yasevich wrote:
> >>>When the user turns off IFF_VNET_HDR flag, attempts to change
> >>>offload features via TUNSETOFFLOAD do not work. This could cause
> >>>GSO packets to be delivered to the user when the user is
> >>>not prepared to handle them.
> >>>
> >>>To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> >>>disabled and make sure to turn off all offloads in this case.
> >>>Also, when IFF_VNET_HDR is disabled, run throught the offload change
> >>>as well to make sure that the functionality is not dependent on
> >>>the order of the ioclt() calls.
> >>>
> >>>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>
> >>Hmm, this seems to make it even messier.
> >>So
> >> set TUNSETOFFLOAD
> >> clear IFF_VNET_HDR
> >> set IFF_VNET_HDR
> >>
> >>offloads are now clear?
> >>
> >>I'm also not sure userspace that sets offloads but clears
> >>vnet hdr knows what it's doing.
> >>
> >>How about we fail attempts to clear vnet hdr
> >>unless offloads are disabled?
> >>
> >
> >OK. That makes it simpler...
>
> Spoke too soon.
>
> What happens is that libvirt is typically the one that turns off
> IFF_VNET_HDR, and qemu controls the offloads.
>
> So, if we forbid turning off vnet_hdr while offloads are set, that
> will break libvirt (this is because macvtap defaults to tap offloads
> being enabled).
>
> -vlad
Well what prevents libvirt from disabling offloads?
Anyway, another approach is to say that this is up to user -
if user disables hdr but not offloads, user will get
packets without a checksum.
> >
> >-vlad
> >
> >>
> >>
> >>
> >>>---
> >>> drivers/net/macvtap.c | 27 +++++++++++++++++++--------
> >>> 1 file changed, 19 insertions(+), 8 deletions(-)
> >>>
> >>>diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> >>>index a98fb0e..076b9e7 100644
> >>>--- a/drivers/net/macvtap.c
> >>>+++ b/drivers/net/macvtap.c
> >>>@@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q,
> >>>unsigned long arg)
> >>> struct macvlan_dev *vlan;
> >>> netdev_features_t features;
> >>> netdev_features_t feature_mask = 0;
> >>>+ netdev_features_t tap_mask = TUN_OFFLOADS;
> >>>
> >>> vlan = rtnl_dereference(q->vlan);
> >>> if (!vlan)
> >>>@@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue
> >>>*q, unsigned long arg)
> >>>
> >>> features = vlan->dev->features;
> >>>
> >>>- if (arg & TUN_F_CSUM) {
> >>>+ if (!(q->flags & IFF_VNET_HDR)) {
> >>>+ /* Turn off all checsum offloading also if user does
> >>>+ * not user vnet_hdr.
> >>
> >>Does not set?
> >>
> >>>+ */
> >>>+ tap_mask |= NETIF_F_ALL_CSUM;
> >>>+ } else if (arg & TUN_F_CSUM) {
> >>> feature_mask = NETIF_F_HW_CSUM;
> >>>
> >>> if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
> >>>@@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q,
> >>>unsigned long arg)
> >>> /* tap_features are the same as features on tun/tap and
> >>> * reflect user expectations.
> >>> */
> >>>- vlan->tap_features = vlan->dev->features &
> >>>- (feature_mask | ~TUN_OFFLOADS);
> >>>+ vlan->tap_features = vlan->dev->features & (feature_mask |
> >>>~tap_mask);
> >>> vlan->set_features = features;
> >>> netdev_update_features(vlan->dev);
> >>>
> >>>@@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file,
> >>>unsigned int cmd,
> >>> if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
> >>> (IFF_NO_PI | IFF_TAP))
> >>> ret = -EINVAL;
> >>>- else
> >>>+ else {
> >>>+ if ((q->flags ^ u) & IFF_VNET_HDR) {
> >>>+ /* vnet_hdr support impacts the offloads,
> >>>+ * so we need to run throught the offload
> >>
> >>throught ?
> >>
> >>>+ * change.
> >>>+ */
> >>>+ rtnl_lock();
> >>>+ ret = set_offload(q, 0);
> >>>+ rtnl_unlock();
> >>>+ }
> >>> q->flags = u;
> >>>+ }
> >>>
> >>> return ret;
> >>>
> >>>@@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file,
> >>>unsigned int cmd,
> >>> TUN_F_TSO_ECN | TUN_F_UFO))
> >>> return -EINVAL;
> >>>
> >>>- /* TODO: only accept frames with the features that
> >>>- got enabled for forwarded frames */
> >>>- if (!(q->flags & IFF_VNET_HDR))
> >>>- return -EINVAL;
> >>> rtnl_lock();
> >>> ret = set_offload(q, arg);
> >>> rtnl_unlock();
> >>>--
> >>>1.8.1.4
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-14 17:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-13 17:55 [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled Vlad Yasevich
2013-08-13 18:05 ` Sergei Shtylyov
2013-08-14 12:10 ` Michael S. Tsirkin
2013-08-14 14:07 ` Vlad Yasevich
2013-08-14 16:04 ` Vlad Yasevich
2013-08-14 17:43 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).