| Message ID | 1355227436-18383-2-git-send-email-amirv@mellanox.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 12/11/2012 04:03 AM, Amir Vadai wrote: > From: Yan Burman <yanb@mellanox.com> > > Add ability to specify destination MAC address for L3/L4 flow spec > in order to be able to specify action for different VM's under vSwitch > configuration. This change is transparent to older userspace. > > Signed-off-by: Yan Burman <yanb@mellanox.com> > Signed-off-by: Amir Vadai <amirv@mellanox.com> > --- > include/uapi/linux/ethtool.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index d3eaaaf..be8c41e 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -500,13 +500,15 @@ union ethtool_flow_union { > struct ethtool_ah_espip4_spec esp_ip4_spec; > struct ethtool_usrip4_spec usr_ip4_spec; > struct ethhdr ether_spec; > - __u8 hdata[60]; > + __u8 hdata[52]; > }; > > struct ethtool_flow_ext { > - __be16 vlan_etype; > - __be16 vlan_tci; > - __be32 data[2]; > + __u8 padding[2]; > + unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ > + __be16 vlan_etype; > + __be16 vlan_tci; > + __be32 data[2]; > }; > Is there any special reason why you need to change the size of this structure? It seems like you could probably just replace the data section with a union containing either 8 bytes of user specified data or your MAC address data. Then we wouldn't need all of the changes to the rest of the flow specifier. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 11, 2012 at 7:37 PM, Alexander Duyck <alexander.h.duyck@intel.com> wrote: > > Is there any special reason why you need to change the size of this > structure? It seems like you could probably just replace the data > section with a union containing either 8 bytes of user specified data or > your MAC address data. Then we wouldn't need all of the changes to the > rest of the flow specifier. basically, we followed Ben's suggestions made here http://marc.info/?t=134977576500003 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/11/2012 11:55 AM, Or Gerlitz wrote: > On Tue, Dec 11, 2012 at 7:37 PM, Alexander Duyck > <alexander.h.duyck@intel.com> wrote: >> >> Is there any special reason why you need to change the size of this >> structure? It seems like you could probably just replace the data >> section with a union containing either 8 bytes of user specified data or >> your MAC address data. Then we wouldn't need all of the changes to the >> rest of the flow specifier. > > > basically, we followed Ben's suggestions made here > http://marc.info/?t=134977576500003 > After looking over Ben's suggestion it makes sense. Since I am pretty much responsible for the ixgbe implementation of this I will do a few quick tests once the patches are applied to make certain that there were no issues introduced on our end. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2012-12-11 at 14:03 +0200, Amir Vadai wrote: > From: Yan Burman <yanb@mellanox.com> > > Add ability to specify destination MAC address for L3/L4 flow spec > in order to be able to specify action for different VM's under vSwitch > configuration. This change is transparent to older userspace. > > Signed-off-by: Yan Burman <yanb@mellanox.com> > Signed-off-by: Amir Vadai <amirv@mellanox.com> > --- > include/uapi/linux/ethtool.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index d3eaaaf..be8c41e 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -500,13 +500,15 @@ union ethtool_flow_union { > struct ethtool_ah_espip4_spec esp_ip4_spec; > struct ethtool_usrip4_spec usr_ip4_spec; > struct ethhdr ether_spec; > - __u8 hdata[60]; > + __u8 hdata[52]; > }; > > struct ethtool_flow_ext { > - __be16 vlan_etype; > - __be16 vlan_tci; > - __be32 data[2]; > + __u8 padding[2]; > + unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ > + __be16 vlan_etype; > + __be16 vlan_tci; > + __be32 data[2]; > }; > > /** > @@ -1027,6 +1029,7 @@ enum ethtool_sfeatures_retval_bits { > #define ETHER_FLOW 0x12 /* spec only (ether_spec) */ > /* Flag to enable additional fields in struct ethtool_rx_flow_spec */ > #define FLOW_EXT 0x80000000 > +#define FLOW_MAC_EXT 0x40000000 You'll need to document exactly which flags and fields are related. Adding kernel-doc to struct ethtool_flow_ext is probably the best way to do that. Ben. > /* L3-L4 network traffic flow hash options */ > #define RXH_L2DA (1 << 1)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index d3eaaaf..be8c41e 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -500,13 +500,15 @@ union ethtool_flow_union { struct ethtool_ah_espip4_spec esp_ip4_spec; struct ethtool_usrip4_spec usr_ip4_spec; struct ethhdr ether_spec; - __u8 hdata[60]; + __u8 hdata[52]; }; struct ethtool_flow_ext { - __be16 vlan_etype; - __be16 vlan_tci; - __be32 data[2]; + __u8 padding[2]; + unsigned char h_dest[ETH_ALEN]; /* destination eth addr */ + __be16 vlan_etype; + __be16 vlan_tci; + __be32 data[2]; }; /** @@ -1027,6 +1029,7 @@ enum ethtool_sfeatures_retval_bits { #define ETHER_FLOW 0x12 /* spec only (ether_spec) */ /* Flag to enable additional fields in struct ethtool_rx_flow_spec */ #define FLOW_EXT 0x80000000 +#define FLOW_MAC_EXT 0x40000000 /* L3-L4 network traffic flow hash options */ #define RXH_L2DA (1 << 1)