* Re: [PATCH]extensions/tos_values.c mask value not accurate in certain condition
2010-11-02 5:26 [PATCH]extensions/tos_values.c mask value not accurate in certain condition DuanZhenzhong
@ 2010-11-02 8:20 ` Jan Engelhardt
2010-11-09 14:45 ` Patrick McHardy
0 siblings, 1 reply; 3+ messages in thread
From: Jan Engelhardt @ 2010-11-02 8:20 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Developer Mailing List, Joe Jin, DuanZhenzhong
On Tuesday 2010-11-02 06:26, DuanZhenzhong wrote:
>scene:
># iptables -V
>iptables v1.4.10
># iptables -v -t mangle -A MANGLE_OUTPUT -p tcp --dport 20 -j TOS
>--set-tos 8
>TOS tcp opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 tcp dpt:20 TOS set
>0x08/0xff
># iptables -v -t mangle -A MANGLE_OUTPUT -p tcp --dport 20 -j TOS
>--set-tos Maximize-Throughput
>TOS tcp opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 tcp dpt:20 TOS set
>0x08/0x3f
>
>mask value is different for the same tos value. This is because below
>code piece:
>static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
>unsigned int bits)
>{
>const unsigned int max = (1 << bits) - 1;
>......
>tvm->mask = max;
>......
>static bool tos_parse_symbolic(const char *str, struct tos_value_mask *tvm,
>unsigned int def_mask)
>{
>const unsigned int max = UINT8_MAX;
>const struct tos_symbol_info *symbol;
>char *tmp;
>
>if (xtables_strtoui(str, &tmp, NULL, 0, max))
>return tos_parse_numeric(str, tvm, max);
>
>/* Do not consider ECN bits */
>tvm->mask = def_mask;
>.......
>For tos value 8, bits shift lead to a overflow and trim, so the mask is
>0xff no matter what the def_mask is.
>For tos symbol Maximize-Throughput, tvm->mask got def_mask 0x3f.
This at least is intended.
Patrick, please grab from
git://dev.medozas.de/iptables master
parent 600f38db82548a683775fd89b6e136673e924097 (v1.4.9-24-g600f38d)
commit 648fd1ad68ae2ec675ac07efee80783912535404
Author: Jan Engelhardt <jengelh@medozas.de>
Date: Tue Nov 2 09:10:34 2010 +0100
libxt_TOS: avoid an undesired overflowing computation
The @bits parameter was wrongly labeled and should have been @max
already. This makes the - overflowing - 1<<bits redundant of course.
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
extensions/tos_values.c | 19 +++++++++++--------
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/extensions/tos_values.c b/extensions/tos_values.c
index 10add19..a65ef25 100644
--- a/extensions/tos_values.c
+++ b/extensions/tos_values.c
@@ -26,15 +26,13 @@ static const struct tos_symbol_info {
/*
* tos_parse_numeric - parse sth. like "15/255"
*
- * @s: input string
- * @info: accompanying structure
- * @bits: number of bits that are allowed
- * (8 for IPv4 TOS field, 4 for IPv6 Priority Field)
+ * @str: input string
+ * @tvm: (value/mask) tuple
+ * @max: maximum allowed value (must be pow(2,some_int)-1)
*/
static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
- unsigned int bits)
+ unsigned int max)
{
- const unsigned int max = (1 << bits) - 1;
unsigned int value;
char *end;
@@ -56,17 +54,22 @@ static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
return true;
}
+/**
+ * @str: input string
+ * @tvm: (value/mask) tuple
+ * @def_mask: mask to force when a symbolic name is used
+ */
static bool tos_parse_symbolic(const char *str, struct tos_value_mask *tvm,
unsigned int def_mask)
{
- const unsigned int max = UINT8_MAX;
+ static const unsigned int max = UINT8_MAX;
const struct tos_symbol_info *symbol;
char *tmp;
if (xtables_strtoui(str, &tmp, NULL, 0, max))
return tos_parse_numeric(str, tvm, max);
- /* Do not consider ECN bits */
+ /* Do not consider ECN bits when using preset names */
tvm->mask = def_mask;
for (symbol = tos_symbol_names; symbol->name != NULL; ++symbol)
if (strcasecmp(str, symbol->name) == 0) {
--
# Created with git-export-patch
^ permalink raw reply related [flat|nested] 3+ messages in thread