| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4DB9D0D9.2000401@intel.com>
Date: Thu, 28 Apr 2011 13:40:57 -0700
From: Alexander Duyck <alexander.h.duyck@...el.com>
To: Ben Hutchings <bhutchings@...arflare.com>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [ethtool PATCH 6/6] Update documentation for -u/-U operations
On 4/27/2011 11:23 AM, Ben Hutchings wrote:
> On Thu, 2011-04-21 at 13:40 -0700, Alexander Duyck wrote:
>> This patch updates the documentation for the -u/-U operations to include
>> the recent changes made to support addition/deletion/display of network
>> flow classifier rules.
>
> This should be included in the same patch.
I'll combine the two patches for the next release if that is what is
preferred. I was just trying to keep the overall size down by splitting
them since this is documentation only.
>> Signed-off-by: Alexander Duyck<alexander.h.duyck@...el.com>
>> ---
>>
>> ethtool.8.in | 185 +++++++++++++++++++++++++++++-----------------------------
>> ethtool.c | 32 ++++++----
>> 2 files changed, 111 insertions(+), 106 deletions(-)
>>
>> diff --git a/ethtool.8.in b/ethtool.8.in
>> index 12a1d1d..8908351 100644
>> --- a/ethtool.8.in
>> +++ b/ethtool.8.in
>> @@ -42,10 +42,20 @@
>> [\\fB\\$1\\fP\ \\fIN\\fP]
>> ..
>> .\"
>> +.\" .BM - same as above but has a mask field for format "[value N [value-mask N]]"
>> +.\"
>> +.de BM
>> +[\\fB\\$1\\fP\ \\fIN\\fP\ [\\fB\\$1\-mask\\fP\ \\fIN\\fP]]
>
> You've changed the code to accept 'm' as an alternative to
> <field> '-mask', so this should be changed accordingly.
What would be the preferred way of stating that? For now I just
replaced the \\$1\-mask with m. However I am assuming that probably
isn't the best approach either. Should I state somewhere that m can be
replaced with "field name"-mask?
> [...]
>> @@ -236,9 +252,9 @@ ethtool \- query or control network driver and hardware settings
>> .HP
>> .B ethtool \-N
>> .I ethX
>> -.RB [ rx\-flow\-hash \ \*(FL
>> -.RB \ \*(HO]
>> +.RB [ rx-flow-hash \ \*(FL \ \*(HO]
>> .HP
>> +
>
> This looks like an unintentional reversion of part of commit
> db6c0cee6cd956767e1c39109fe81104cc4694cb.
Yeah, my bad. I will have it updated for the next patch. I only really
meant to combine this into one line, I didn't intend to drop the "\-"
formatting fixes you added.
>> .B ethtool \-x|\-\-show\-rxfh\-indir
>> .I ethX
>> .HP
>> @@ -257,54 +273,28 @@ ethtool \- query or control network driver and hardware settings
>> .HP
>> .B ethtool \-u|\-\-show\-ntuple
>> .I ethX
>> -.TP
>> +.BN class-rule
>> +.HP
>> +
>> .BI ethtool\ \-U|\-\-config\-ntuple \ ethX
>> -.RB {
>> -.A3 flow\-type tcp4 udp4 sctp4
>> -.RB [ src\-ip
>> -.IR addr
>> -.RB [ src\-ip\-mask
>> -.IR mask ]]
>> -.RB [ dst\-ip
>> -.IR addr
>> -.RB [ dst\-ip\-mask
>> -.IR mask ]]
>> -.RB [ src\-port
>> -.IR port
>> -.RB [ src\-port\-mask
>> -.IR mask ]]
>> -.RB [ dst\-port
>> -.IR port
>> -.RB [ dst\-port\-mask
>> -.IR mask ]]
>> -.br
>> -.RB | \ flow\-type\ ether
>> -.RB [ src
>> -.IR mac\-addr
>> -.RB [ src\-mask
>> -.IR mask ]]
>> -.RB [ dst
>> -.IR mac\-addr
>> -.RB [ dst\-mask
>> -.IR mask ]]
>> -.RB [ proto
>> -.IR N
>> -.RB [ proto\-mask
>> -.IR mask ]]\ }
>> -.br
>> -.RB [ vlan
>> -.IR VLAN\-tag
>> -.RB [ vlan\-mask
>> -.IR mask ]]
>> -.RB [ user\-def
>> -.IR data
>> -.RB [ user\-def\-mask
>> -.IR mask ]]
>> -.RI action \ N
>> -.
>> -.\" Adjust lines (i.e. full justification) and hyphenate.
>> -.ad
>> -.hy
>
> As do the last 3 deleted lines here.
I put them back so they should be left in place for the next version.
>> +.BN class-rule-del
>> +.RB [\ flow-type \ \*(NC
>> +.RB [ src \ \*(MA\ [ src-mask \ \*(MA]]
>> +.RB [ dst \ \*(MA\ [ dst-mask \ \*(MA]]
>> +.BM proto
>> +.RB [ src-ip \ \*(PA\ [ src-ip-mask \ \*(PA]]
>> +.RB [ dst-ip \ \*(PA\ [ dst-ip-mask \ \*(PA]]
>> +.BM tos
>> +.BM l4proto
>> +.BM src-port
>> +.BM dst-port
>> +.BM spi
>> +.BM vlan-etype
>> +.BM vlan
>> +.BM user-def
>> +.BN action
>> +.BN loc
>> +.RB ]
>
> But these options aren't all applicable to all flow-types.
This is the part that gets messy and I am not sure what the best
approach is. I have more comments on that below. For now what I am
planning to implement to address this is that in the "DESCRIPTION"
section below I add a statement to each specifier that has restrictions
by stating "Valid for flow-types X, Y, and Z."
> [...]
>> diff --git a/ethtool.c b/ethtool.c
>> index 421fe20..e65979d 100644
>> --- a/ethtool.c
>> +++ b/ethtool.c
>> @@ -243,20 +243,26 @@ static struct option {
>> " equal N | weight W0 W1 ...\n" },
>> { "-U", "--config-ntuple", MODE_SCLSRULE, "Configure Rx ntuple filters "
>> "and actions",
>> - " { flow-type tcp4|udp4|sctp4\n"
>> - " [ src-ip ADDR [src-ip-mask MASK] ]\n"
>> - " [ dst-ip ADDR [dst-ip-mask MASK] ]\n"
>> - " [ src-port PORT [src-port-mask MASK] ]\n"
>> - " [ dst-port PORT [dst-port-mask MASK] ]\n"
>> - " | flow-type ether\n"
>> - " [ src MAC-ADDR [src-mask MASK] ]\n"
>> - " [ dst MAC-ADDR [dst-mask MASK] ]\n"
>> - " [ proto N [proto-mask MASK] ] }\n"
>> - " [ vlan VLAN-TAG [vlan-mask MASK] ]\n"
>> - " [ user-def DATA [user-def-mask MASK] ]\n"
>> - " action N\n" },
>> + " [ class-rule-del %d ] |\n"
>> + " [ flow-type ether|ip4|tcp4|udp4|sctp4|ah4|esp4\n"
>> + " [ src %x:%x:%x:%x:%x:%x [src-mask %x:%x:%x:%x:%x:%x] ]\n"
>> + " [ dst %x:%x:%x:%x:%x:%x [dst-mask %x:%x:%x:%x:%x:%x] ]\n"
>> + " [ proto %d [proto-mask MASK] ]\n"
>> + " [ src-ip %d.%d.%d.%d [src-ip-mask %d.%d.%d.%d] ]\n"
>> + " [ dst-ip %d.%d.%d.%d [dst-ip-mask %d.%d.%d.%d] ]\n"
>> + " [ tos %d [tos-mask %x] ]\n"
>> + " [ l4proto %d [l4proto-mask MASK] ]\n"
>> + " [ src-port %d [src-port-mask %x] ]\n"
>> + " [ dst-port %d [dst-port-mask %x] ]\n"
>> + " [ spi %d [spi-mask %x] ]\n"
>> + " [ vlan-etype %x [vlan-etype-mask %x] ]\n"
>> + " [ vlan %x [vlan-mask %x] ]\n"
>> + " [ user-def %x [user-def-mask %x] ]\n"
>> + " [ action %d ]\n"
>> + " [ loc %d]]\n" },
> [...]
>
> Again, it's not clear which options apply to which flow-types, and the
> 'm' shortcut is not documented.
The 'm' part I agree with 100%, however the flow types are going to
become kinda hairy using that approach. You basically end up with
something like this:
flow-type ether
[ src %x:%x:%x:%x:%x:%x [src-mask %x:%x:%x:%x:%x:%x]]
[ dst %x:%x:%x:%x:%x:%x [dst-mask %x:%x:%x:%x:%x:%x]]
[ proto %d [proto-mask %x]]
[ vlan-etype %x [vlan-etype-mask %x]]
[ vlan %x [vlan-mask %x]]
[ user-def %x [user-def-mask %x]]
[ action %d ]
[ loc %d]
flow-type ip4
[ src-ip %d.%d.%d.%d [src-ip-mask %d.%d.%d.%d]]
[ dst-ip %d.%d.%d.%d [dst-ip-mask %d.%d.%d.%d]]
[ tos %d [tos-mask %x]]
[ l4proto %d [l4proto-mask %x]]
[ src-port %d [src-port-mask %x]]
[ dst-port %d [dst-port-mask %x]]
[ spi %d [spi-mask %x]]
[ vlan-etype %x [vlan-etype-mask %x]]
[ vlan %x [vlan-mask %x]]
[ user-def %x [user-def-mask %x]]
[ action %d ]
[ loc %d]
flow-type tcp4|udp4|sctp4
[ src-ip %d.%d.%d.%d [src-ip-mask %d.%d.%d.%d]]
[ dst-ip %d.%d.%d.%d [dst-ip-mask %d.%d.%d.%d]]
[ tos %d [tos-mask %x]]
[ src-port %d [src-port-mask %x]]
[ dst-port %d [dst-port-mask %x]]
[ vlan-etype %x [vlan-etype-mask %x]]
[ vlan %x [vlan-mask %x]]
[ user-def %x [user-def-mask %x]]
[ action %d ]
[ loc %d]
flow-type ah4|esp4
[ src-ip %d.%d.%d.%d [src-ip-mask %d.%d.%d.%d]]
[ dst-ip %d.%d.%d.%d [dst-ip-mask %d.%d.%d.%d]]
[ tos %d [tos-mask %x]]
[ spi %d [spi-mask %x]]
[ vlan-etype %x [vlan-etype-mask %x]]
[ vlan %x [vlan-mask %x]]
[ user-def %x [user-def-mask %x]]
[ action %d ]
[ loc %d]
As you can see it will be a bit oversized to go through and specify
which flow-type options support what fields. If that is what you want I
can implement it that way but for now I would prefer calling out the
flow-type limitations of the fields in the "DESCRIPTION" portion of the
man page.
>
> Ben.
>
Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists