* syn connection not replied with ack
From: mpe @ 2012-08-28 17:03 UTC (permalink / raw)
To: netdev
Hi all,
I have very very strange behaviour with a virtual machine (centos 6
up-to-date).
Then I try to connect to it with a linux ssh client, I see the syn, but
not the syn-ack replied. On the same target when I connect with windows
machine, it reply correctly and the communication starts.
tshark show me with linux:
386.134073 172.18.0.160 -> 172.18.0.254 TCP 45092 > ssh [SYN] Seq=0
Win=4240 Len=0 MSS=1060 TSV=497047 TSER=0 WS=5
389.134556 172.18.0.160 -> 172.18.0.254 TCP 45092 > ssh [SYN] Seq=0
Win=4240 Len=0 MSS=1060 TSV=497798 TSER=0 WS=5
395.134926 172.18.0.160 -> 172.18.0.254 TCP 45092 > ssh [SYN] Seq=0
Win=4240 Len=0 MSS=1060 TSV=499298 TSER=0 WS=5
and windows:
1099.091686 172.18.0.141 -> 172.18.0.254 TCP 32906 > ssh [SYN] Seq=0
Win=8192 Len=0 MSS=1260 WS=8
1099.092934 172.18.0.254 -> 172.18.0.141 TCP ssh > 32906 [SYN, ACK]
Seq=0 Ack=1 Win=5840 Len=0 MSS=1380
1099.092960 172.18.0.141 -> 172.18.0.254 TCP 32906 > ssh [ACK] Seq=1
Ack=1 Win=65520 Len=0
1099.102017 172.18.0.254 -> 172.18.0.141 SSH Server Protocol:
SSH-2.0-OpenSSH_4.3
iptables are disabled, no modules active (lsmod | grep ip show nothing)
I tried also to remove the vmware vmxnet3 module, reloaded, network
restart, but the same.
But... if I reboot, the communication start for the next 2/3 days and
after it stops another time.
Hope that someone can help me.
Michele
^ permalink raw reply
* Re: [PATCH] iproute2: tc.8: update UNITS section.
From: Ben Hutchings @ 2012-08-28 16:54 UTC (permalink / raw)
To: Li Wei; +Cc: shemminger, netdev
In-Reply-To: <1346132337-21325-1-git-send-email-lw@cn.fujitsu.com>
On Tue, 2012-08-28 at 13:38 +0800, Li Wei wrote:
> - rename section UNITS to PARAMETERS.
> - break section PARAMETERS down to four subsections to cover the
> common used parameter types(RATES, TIMES, SIZES, VALUES).
> - add some explaination for IEC units in RATES.
> - point out the max value we can set for RATES, TIMES and SIZES.
>
> Signed-off-by: Li Wei <lw@cn.fujitsu.com>
> ---
> man/man8/tc.8 | 111 +++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 85 insertions(+), 26 deletions(-)
>
> diff --git a/man/man8/tc.8 b/man/man8/tc.8
> index 98fbfcd..44d644a 100644
> --- a/man/man8/tc.8
> +++ b/man/man8/tc.8
> @@ -257,19 +257,20 @@ qdiscs applies.
> FILTERS
> Filters have a three part ID, which is only needed when using a hashed
> filter hierarchy.
> -.SH UNITS
> -All parameters accept a floating point number, possibly followed by a unit.
> -.P
> -Bandwidths or rates can be specified in:
> -.TP
> -bps
> -Bytes per second
> +
> +.SH PARAMETERS
> +These following parameters are widely used in TC, for other parameters,
'These' should be 'The'.
'for other parameters' should begin a new sentence.
> +see the man page for individual qdiscs.
'page' should be 'pages'.
> +
> .TP
> -kbps
> -Kilobytes per second
> +RATES
> +Bandwidths or rates.
> +These parameters accept a floating point number, possibly followed by
> +a unit(both SI and IEC units supported).
Missing space before the open-parenthesis.
> +.RS
> .TP
> -mbps
> -Megabytes per second
> +bit or a bare number
> +Bits per second
> .TP
> kbit
> Kilobits per second
> @@ -277,27 +278,41 @@ Kilobits per second
> mbit
> Megabits per second
> .TP
> -bit or a bare number
> -Bits per second
> -.P
> -Amounts of data can be specified in:
> +gbit
> +Gegabits per second
'Gigabits'
> .TP
> -kb or k
> -Kilobytes
> +tbit
> +Terabits per second
> .TP
> -mb or m
> -Megabytes
> +bps
> +Bytes per second
> .TP
> -mbit
> -Megabits
> +kbps
> +Kilobytes per second
> .TP
> -kbit
> -Kilobits
> +mbps
> +Megabytes per second
> .TP
> -b or a bare number
> -Bytes.
> +gbps
> +Gigabytes per second
> +.TP
> +tbps
> +Terabytes per second
> +
> +.P
> +To specified in IEC units, just replace the SI prefix(k-, m-, g-, t-) with
> +IEC prefix(ki-, mi-, gi- and ti-) respectively.
'specified' should be 'specify'.
The word 'just' is not needed.
Missing spaces before the open-parentheses.
> +
> .P
> -Lengths of time can be specified in:
> +TC store rates as a __u32 integer in bps internally,
> +so we can specify a max rate of UINT32_MAX bps.
> +.RE
> +
> +.TP
> +TIMES
> +Length of time. Can be specified as a float pointing number
'float pointing' should be 'floating point' :-)
> +followed by an optional unit:
> +.RS
> .TP
> s, sec or secs
> Whole seconds
> @@ -308,6 +323,50 @@ Milliseconds
> us, usec, usecs or a bare number
> Microseconds.
>
> +.P
> +TC defined it's own time unit(equals to microsecond) and store time values
'it's' means 'it is'; here it should be 'its'.
'equals' should be 'equal'.
'store' should be 'stores'.
Missing space before the open parenthesis, again.
> +as __u32 integer, thus we can specify a max time value of UINT32_MAX usecs.
I don't think user documentation should use names like '__u32' and
'UINT32_MAX'. Instead it should say '32-bit unsigned integer' and '4294
seconds'.
> +.RE
> +
> +.TP
> +SIZES
> +Amounts of data. Can be specified as a float pointing number
'float pointing' should be 'floating point' again
> +followed by an optional unit:
> +.RS
> +.TP
> +b or a bare number
> +Bytes.
> +.TP
> +kbit
> +Kilobites
'kilobits'
> +.TP
> +kb or k
> +Kilobytes
> +.TP
> +mbit
> +Megabits
> +.TP
> +mb or m
> +Megabytes
> +.TP
> +gbit
> +Gigabites
'gigabits'
> +.TP
> +gb or g
> +Gigabytes
> +
> +.P
> +TC store sizes internally as __u32 integer in byte, so we can specify
> +a max size of UINT32_MAX bytes.
'store' should be 'stores'.
Same issue with the type and maximum names.
> +.RE
> +
> +.TP
> +VALUES
> +Other values without a unit.
> +These parameters read as decimal by default, but you can
'read' should be 'are read' or 'are interpreted'
Ben.
> +indicate TC to read them as octal and hexadecimal by adding a '0'
> +or '0x' prefix respectively.
> +
> .SH TC COMMANDS
> The following commands are available for qdiscs, classes and filter:
> .TP
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v2 03/10] cgroup: net_cls: Protect access to task_cls_classid() when built as module
From: Daniel Wagner @ 2012-08-28 16:41 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Tejun Heo, netdev-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA, David S. Miller, Gao feng,
Jamal Hadi Salim, John Fastabend, Li Zefan, Neil Horman
In-Reply-To: <20120828144725.GR2961-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
On Tue, Aug 28, 2012 at 07:47:25AM -0700, Paul E. McKenney wrote:
> On Sat, Aug 25, 2012 at 06:56:29PM +0200, Daniel Wagner wrote:
> > On 25.08.2012 01:26, Tejun Heo wrote:
> > >On Fri, Aug 24, 2012 at 04:01:37PM +0200, Daniel Wagner wrote:
> > >>@@ -306,6 +312,11 @@ static void __exit exit_cgroup_cls(void)
> > >> synchronize_rcu();
> > >> #endif
> > >>
> > >>+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
> > >>+ static_key_slow_dec(&cgroup_cls_enabled);
> > >>+ rcu_barrier();
> > >
> > >Why is this rcu_barrier() necessary?
> >
> > I have read the rcubarrier.txt document and I got from that that an
> > rcu_barrier() is needed when unloading a module. But maybe I got it
> > wrong.
> >
> > So the idea after disabling the jump lables all pending readers in
> > task_cls_classid() have left. THe same thing is done in the old code
> > with the dynamic id part. With the difference that synchronize_rcu()
> > is used.
>
> FWIW, the rcu_barrier() is needed only if the module uses call_rcu().
> In that case it is required to ensure that all the resulting callbacks
> execute before the module's .text is freed up.
Thanks for the clarification. After reading the documentations again
and I think this here should do the trick:
static inline u32 task_cls_classid(struct task_struct *p)
{
struct cgroup_cls_state *cs;
u32 classid = 0;
if (!clscg_enabled || in_interrupt())
return 0;
rcu_read_lock();
cs = container_of(task_subsys_state(p, net_cls_subsys_id),
struct cgroup_cls_state, css);
if (cs)
classid = cs->classid;
rcu_read_unlock();
return classid;
}
static void __exit exit_cgroup_cls(void)
{
unregister_tcf_proto_ops(&cls_cgroup_ops);
#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
static_key_slow_dec(&cgroup_cls_enabled);
synchronize_rcu();
#endif
cgroup_unload_subsys(&net_cls_subsys);
}
So when unloading the module, we first disable the static branch, then
we wait for all old readers leaving the reader side issuing a
synchronize_rcu(). New readers might already have passed the static
branch and now entering the reader side. So we need to test if the
pointer we retrieve is valid. Basically, this change is using the same
approach we had before. Instead looking at the id is valid we look at
the pointer if it is valid.
^ permalink raw reply
* Warning in kernel/workqueue.c:1225 worker_enter_idle+0xc7/0x120() when HTB shaper is initialized on top of ifb
From: Andrew @ 2012-08-28 14:26 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]
Hi all.
I tested distro with 3.2.28 kernel on Phenom II 955 at MS-7596 mobo, and I saw
next warning:
------------[ cut here ]------------
WARNING: at
/var/testpoint/LEAF-new/source/i486-unknown-linux-uclibc/linux/linux-3.2.28/kernel/workqueue.c:1225
worker_enter_idle+0xc7/0x120()
Hardware name: MS-7596
Modules linked in: ipv6 k10temp sp5100_tco i2c_piix4 i2c_core
snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_pcm snd_timer igb(O) snd
shpchp dca soundcore pci_hotplug snd_page_alloc powernow_k8 freq_table
processor thermal_sys hwmon mperf act_mirred cls_u32 cls_fw sch_ingress
sch_prio sch_sfq sch_htb ifb 8021q softdog pptp gre sha1_generic arc4 ecb
ppp_mppe pppoe pppox ppp_synctty ppp_generic slhc sd_mod pata_atiixp pata_acpi
ahci libahci ata_generic ehci_hcd ohci_hcd libata usbcore scsi_mod usb_common
pcspkr vfat fat isofs
Pid: 325, comm: kworker/1:1 Tainted: G O 3.2.28-i686 #1
Call Trace:
[<81032dc8>] ? warn_slowpath_common+0x78/0xb0
[<810453b7>] ? worker_enter_idle+0xc7/0x120
[<810453b7>] ? worker_enter_idle+0xc7/0x120
[<81032e1b>] ? warn_slowpath_null+0x1b/0x20
[<810453b7>] ? worker_enter_idle+0xc7/0x120
[<81047417>] ? worker_thread+0x1b7/0x2b0
[<81047260>] ? manage_workers.isra.31+0x1c0/0x1c0
[<8104aba9>] ? kthread+0x69/0x70
[<8104ab40>] ? kthread_worker_fn+0x100/0x100
[<8127c47e>] ? kernel_thread_helper+0x6/0xd
---[ end trace ac1da733157f3bc5 ]---
It was caused during shaper initialization (creating HTB hierarchical shaper on
ifb0 device, with hash filter table for 3 /22 networks, 3 classes per IP), and
appears only once, at start of shaper init (shaper init takes ~1 minute).
It is reproducible, PID is almost every time the same (just once it was 62xx and
points, as I remember, to kworker/2:1).
Kernel config, script that initializes shaper and script config is in attach.
System has, among other mentioned hardware, 2x2GB DDR3 RAM, and i82576-based
LAN card that uses igb-3.3.6 driver.
Distro is LEAF (http://leaf.sourceforge.net), development branch from git.
Used gcc version is 4.6.3; binutils - 2.21a or 2.22 (I can't remember which one
is actually used for that build - we switched for 2.22 and back to 2.21 after
errors caused by binutils update, then - again to 2.22).
If needed I can place tarball that was used on this PC (~90MB).
I placed this bug at bugzilla (bug 46501), and received recommendation to ask here.
[-- Attachment #2: hsh.conf --]
[-- Type: text/plain, Size: 400 bytes --]
tc="/sbin/tc"
ip="/sbin/ip"
sed="/bin/sed"
awk="/usr/bin/awk"
POOLS="91.202.132.0/22 91.226.56.0/22 10.251.0.0/22"
UPDEVS="ifb0 vlan1 vlan2"
UPDEVINIT="$ip l s ifb0 up 2>/dev/null"
UPDEVSTOP="$tc q d root dev ifb0 2>/dev/null; $ip l s ifb0 down 2>/dev/null;\
$tc q d root dev vlan1 2>/dev/null; $tc q d root dev vlan2 2>/dev/null"
URATE=1000Mbit
UHRATE=500Mbit
UCEIL=1000Mbit
UBURST="burst 256k"
[-- Attachment #3: hsh.sh --]
[-- Type: text/plain, Size: 5620 bytes --]
#!/bin/sh
. /etc/hsh.conf
#convert ip to integer
ip2int() {
echo $@|$sed 's/\(\.\|\/\)/ /g'|$awk '{if ((NF==4)||(NF==5)) {OFMT = "%.0f";
print $1*2^24+$2*2^16+$3*2^8+$4}}'
}
#convert integer to ip
int2ip() {
echo $@|$awk '{if (NF==1) {s1=$1%2^24; s2=$1%2^16; OFMT = "%.0f";\
print int($1/2^24)"."int(s1/2^16)"."int(s2/2^8)"."$1%256}}'
}
#get subnet addr for ip/mask
subnet() {
echo $@|$sed 's/\(\.\|\/\)/ /g'|$awk '{if (NF==5) {ip=$1*2^24+$2*2^16+$3*2^8+$4; \
subnet=ip-ip%2^(32-$5); s1=subnet%2^24; s2=subnet%2^16; OFMT = "%.0f";\
print int(subnet/2^24)"."int(s1/2^16)"."int(s2/2^8)"."subnet%256"/"$5}}'
}
#get width of subnet
netwidth() {
echo $@|$sed 's/\(\.\|\/\)/ /g'|$awk '{if (NF==5) {OFMT = "%.0f"; print 2^(32-$5)}}'
}
#get # of 256-byte subnets of network
subcnt() {
echo $@|$sed 's/\(\.\|\/\)/ /g'|$awk '{if (NF==5) {width=2^(32-$5); i=(width%256>0);
OFMT = "%.0f"; print int(width/256)+i }}'
}
#return max of 2 ints
max() {
echo $@|$awk '{OFMT = "%.0f"; if (NF==2) {if ($1>$2) {print $1} else {print $2}}}'
}
#print as hex
hex() {
echo $@|$awk '{if (NF==1) {printf "%x", $1}}'
}
#sequence from $1 to $1+$2-1
lseq() {
echo $@|$awk '{OFMT = "%.0f"; if (NF==2) {for (i=0;i<$2;i++) {print $1+i}}}'
}
#init device $1
initdev() {
echo $UPDEVINIT|sh
$tc q d root dev $1 2>/dev/null
$tc q a root dev $1 handle 1: htb default 2
$tc c a dev $1 parent 1: classid 1:1 htb rate $URATE ceil $UCEIL $UBURST prio 1 quantum 1514
$tc c a dev $1 parent 1: classid 1:2 htb rate $URATE ceil $UCEIL $UBURST prio 2 quantum 1514
$tc q a dev $1 parent 1:2 handle 2: sfq perturb 10 quantum 1514
$tc f a dev $1 parent 1: prio 10 protocol ip u32
}
#add rule $1 with addr $2 for table $3 with default speed on device $4
addrule() {
ar_id=$(($POOLWIDTH*($3+1)+$1))
ar_id1=$(($RULECOUNT+$ar_id))
ar_id2=$(($RULECOUNT*2+$ar_id))
ar_h1=$(hex $1)
$tc c a dev $4 parent 1:1 classid 1:$ar_id htb rate $URATE ceil $UCEIL $UBURST prio 1 quantum 1514
$tc c a dev $4 parent 1:$ar_id classid 1:$ar_id1 htb rate $UHRATE ceil $UCEIL $UBURST prio 2 quantum 1514
$tc c a dev $4 parent 1:$ar_id classid 1:$ar_id2 htb rate $UHRATE ceil $UCEIL $UBURST prio 1 quantum 1514
$tc q a dev $4 parent 1:$ar_id1 handle $ar_id1: sfq perturb 10 quantum 1514
$tc q a dev $4 parent 1:$ar_id2 handle $ar_id2: sfq perturb 10 quantum 1514
$tc f a dev $4 parent 1: protocol ip prio 1 u32 ht $3:$ar_h1: \
match ip tos 0x10 0xff flowid 1:$ar_id2
$tc f a dev $4 parent 1: protocol ip prio 2 u32 ht $3:$ar_h1: \
match ip protocol 6 0xff match u8 0x45 0xff at 0 match u16 0x0000 0xffc0 at 2 \
match u8 0x10 0xff at 33 flowid 1:$ar_id2
$tc f a dev $4 parent 1: protocol ip prio 3 u32 ht $3:$ar_h1: \
match ip protocol 1 0xff flowid 1:$ar_id2
$tc f a dev $4 parent 1: protocol ip prio 4 u32 ht $3:$ar_h1: match ip src $2 \
flowid 1:$ar_id1
}
#set speed for rule $1 in table $2 with rate $3 kbit on device $4
setspeed() {
ss_id=$(($POOLWIDTH*($2+1)+$1))
ss_id1=$(($RULECOUNT+$ss_id))
ss_id2=$(($RULECOUNT*2+$ss_id))
ss_hr=$(($3/2))
$tc c r dev $4 parent 1:1 classid 1:$ss_id htb rate ${3}kbit $UBURST prio 1 quantum 1514
$tc c r dev $4 parent 1:$ss_id classid 1:$ss_id1 htb rate ${ss_hr}kbit ceil ${3}kbit $UBURST prio 2 quantum 1514
$tc c r dev $4 parent 1:$ss_id classid 1:$ss_id2 htb rate ${ss_hr}kbit ceil ${3}kbit $UBURST prio 1 quantum 1514
}
#fill table for subnet addr $1, device $2
addtable() {
nwidth=$(netwidth $1)
$tc f a dev $2 parent 1:1 prio 10 handle $tctr: protocol ip u32 divisor $nwidth
$tc f a dev $2 parent 1: protocol ip prio 10 u32 ht 800:: \
match ip src $(subnet $1) \
hashkey mask 0x$(hex $(($nwidth-1))) at 12 \
link $tctr:
at_t=0
for at_i in $(lseq $(ip2int $1) $nwidth); do
addrule $at_t $(int2ip $at_i) $tctr $2
at_t=$(($at_t+1))
done
}
#divide network $1 by subnets
divnet() {
for dn_net in $@; do
dn_count=$(subcnt $dn_net)
if [ $dn_count -gt 1 ]; then
dn_subnet=$(ip2int $(subnet $dn_net))
for dn_i in $(lseq 0 $dn_count); do
echo $(int2ip $((dn_subnet+dn_i*256)))/24
done
else
echo $dn_net
fi
done
}
#is ip $2 in subnet $1?
chkip() {
echo $(ip2int $1) $(ip2int $2) $(netwidth $1)|awk '{print (($2>=$1)&&($2<=$1+$3))}'
}
POOLWIDTH=0
NPOOLS=$(divnet $POOLS)
NETCOUNT=$(echo $NPOOLS|wc -w)
for i in $NPOOLS; do
POOLWIDTH=$(max $POOLWIDTH $(netwidth $i))
done
RULECOUNT=$(($POOLWIDTH*$NETCOUNT))
tctr=1
case "$1" in
init)
for iface in $UPDEVS; do
initdev $iface
tctr=1
for i in $NPOOLS; do
addtable $i $iface
tctr=$(($tctr+1))
done
done;;
set)
for i in $NPOOLS; do
if [ $(chkip $i $2) -eq 1 ]; then
rulenum=$(($(ip2int $2)-$(ip2int $i)))
if [ -z "$4" ]; then
for iface in $UPDEVS; do
setspeed $rulenum $tctr $3 $iface
done
else
setspeed $rulenum $tctr $3 $4
fi
exit 0
fi
tctr=$(($tctr+1))
done
echo "This IP isn't in pool!"
exit 1;;
cstat)
for i in $NPOOLS; do
if [ $(chkip $i $2) -eq 1 ]; then
cl1=$(($POOLWIDTH*($tctr+1)+$(ip2int $2)-$(ip2int $i)))
if [ -z "$3" ]; then
for iface in $UPDEVS; do
echo "===== Interface $iface ====="
$tc -s c s dev $iface|grep -A 4 " 1:$cl1 "
done;
else
$tc -s c s dev $3|grep -A 4 " 1:$cl1 "
fi
exit 0
fi
tctr=$(($tctr+1))
done
echo "This IP isn't in pool!"
exit 1;;
stop)
echo $UPDEVSTOP|sh;;
*)
echo Usage: "$0 (init|stop|set <ip> <speed in kbit> [iface]|cstat <ip> [iface])";;
esac
[-- Attachment #4: Bering-3.2.28.config --]
[-- Type: application/xml, Size: 85157 bytes --]
^ permalink raw reply
* Re: [PATCH V2 2/2] ipvs: Extend MTU check to account for IPv6 NAT defrag changes
From: Eric Dumazet @ 2012-08-28 14:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Patrick McHardy, lvs-devel, Julian Anastasov,
Simon Horman, Pablo Neira Ayuso, Hans Schillstrom, Wensong Zhang,
netfilter-devel
In-Reply-To: <20120828142248.11475.15917.stgit@dragon>
On Tue, 2012-08-28 at 16:23 +0200, Jesper Dangaard Brouer wrote:
> This patch is necessary, to make IPVS work, after Patrick McHardys
> IPv6 NAT defragmentation changes.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> In V2: the tunnel mode is no longer a special case.
>
> net/netfilter/ipvs/ip_vs_xmit.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> index 67a3978..56f6d5d 100644
> --- a/net/netfilter/ipvs/ip_vs_xmit.c
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -88,7 +88,14 @@ __ip_vs_dst_check(struct ip_vs_dest *dest, u32 rtos)
> static inline bool
> __mtu_check_toobig_v6(const struct sk_buff *skb, u32 mtu)
> {
> - if (skb->len > mtu && !skb_is_gso(skb)) {
> + if (IP6CB(skb)->frag_max_size) {
> + /* frag_max_size tell us that, this packet have been
> + * defragmented by netfilter IPv6 conntrack module.
> + */
> + if (IP6CB(skb)->frag_max_size > mtu)
> + return true; /* largest fragment violate MTU */
> + }
> + else if (skb->len > mtu && !skb_is_gso(skb)) {
> return true; /* Packet size violate MTU size */
> }
Couldnt you use a single test ?
if (IP6CB(skb)->frag_max_size > mtu)
return true;
if (skb->len > mtu && !skb_is_gso(skb))
return true;
^ permalink raw reply
* Re: [PATCH v2 03/10] cgroup: net_cls: Protect access to task_cls_classid() when built as module
From: Paul E. McKenney @ 2012-08-28 14:47 UTC (permalink / raw)
To: Daniel Wagner
Cc: Tejun Heo, netdev-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA, David S. Miller, Gao feng,
Jamal Hadi Salim, John Fastabend, Li Zefan, Neil Horman
In-Reply-To: <503903BD.6020208-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
On Sat, Aug 25, 2012 at 06:56:29PM +0200, Daniel Wagner wrote:
> On 25.08.2012 01:26, Tejun Heo wrote:
> >On Fri, Aug 24, 2012 at 04:01:37PM +0200, Daniel Wagner wrote:
> >>@@ -306,6 +312,11 @@ static void __exit exit_cgroup_cls(void)
> >> synchronize_rcu();
> >> #endif
> >>
> >>+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
> >>+ static_key_slow_dec(&cgroup_cls_enabled);
> >>+ rcu_barrier();
> >
> >Why is this rcu_barrier() necessary?
>
> I have read the rcubarrier.txt document and I got from that that an
> rcu_barrier() is needed when unloading a module. But maybe I got it
> wrong.
>
> So the idea after disabling the jump lables all pending readers in
> task_cls_classid() have left. THe same thing is done in the old code
> with the dynamic id part. With the difference that synchronize_rcu()
> is used.
FWIW, the rcu_barrier() is needed only if the module uses call_rcu().
In that case it is required to ensure that all the resulting callbacks
execute before the module's .text is freed up.
Thanx, Paul
> >In general, please explain what
> >synchronization is going on when using sync constructs which aren't
> >obvious - e.g. memory barriers, rcu barriers.
>
> Sure, I will keep this in mind.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8192de: remove pointless conditional before kfree_skb()
From: Larry Finger @ 2012-08-28 14:44 UTC (permalink / raw)
To: Wei Yongjun
Cc: chaoming_li-kXabqFNEczNtrwSWzY7KCg,
linville-2XuSBdqkA4R54TAoqtyWWQ,
yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAPgLHd86W5X=Tb_fXoz_Ood-w_s282QNePTXG9m_6ESdG-rG2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 08/28/2012 08:11 AM, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>
> Remove pointless conditional before kfree_skb().
>
> Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
> ---
> drivers/net/wireless/rtlwifi/rtl8192de/fw.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Acked-by: Larry Finger <Larry.Finger.net>
Thanks,
Larry
>
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/fw.c b/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
> index 895ae6c..a3aede0 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
> @@ -570,8 +570,7 @@ static bool _rtl92d_cmd_send_packet(struct ieee80211_hw *hw,
>
> ring = &rtlpci->tx_ring[BEACON_QUEUE];
> pskb = __skb_dequeue(&ring->queue);
> - if (pskb)
> - kfree_skb(pskb);
> + kfree_skb(pskb);
> spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
> pdesc = &ring->desc[idx];
> /* discard output from call below */
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8192c: remove pointless conditional before kfree_skb()
From: Larry Finger @ 2012-08-28 14:44 UTC (permalink / raw)
To: Wei Yongjun; +Cc: chaoming_li, linville, yongjun_wei, linux-wireless, netdev
In-Reply-To: <CAPgLHd_q0T=RD+qgEzFpnhez4Hdu-jDuFf0E+MgwiWLSeaMYaQ@mail.gmail.com>
On 08/28/2012 08:12 AM, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Remove pointless conditional before kfree_skb().
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Acked-by: Larry Finger <Larry.Finger.net>
Thanks,
Larry
>
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c b/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
> index 44febfd..b627151 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
> @@ -577,8 +577,7 @@ static bool _rtl92c_cmd_send_packet(struct ieee80211_hw *hw,
> ring = &rtlpci->tx_ring[BEACON_QUEUE];
>
> pskb = __skb_dequeue(&ring->queue);
> - if (pskb)
> - kfree_skb(pskb);
> + kfree_skb(pskb);
>
> spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
>
>
>
>
^ permalink raw reply
* [PATCH V2 2/2] ipvs: Extend MTU check to account for IPv6 NAT defrag changes
From: Jesper Dangaard Brouer @ 2012-08-28 14:23 UTC (permalink / raw)
To: netdev, Patrick McHardy, lvs-devel, Julian Anastasov,
Simon Horman, Pablo Neira Ayuso
Cc: Jesper Dangaard Brouer, Hans Schillstrom, Wensong Zhang,
netfilter-devel
In-Reply-To: <20120828142051.11475.63775.stgit@dragon>
This patch is necessary, to make IPVS work, after Patrick McHardys
IPv6 NAT defragmentation changes.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
In V2: the tunnel mode is no longer a special case.
net/netfilter/ipvs/ip_vs_xmit.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 67a3978..56f6d5d 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -88,7 +88,14 @@ __ip_vs_dst_check(struct ip_vs_dest *dest, u32 rtos)
static inline bool
__mtu_check_toobig_v6(const struct sk_buff *skb, u32 mtu)
{
- if (skb->len > mtu && !skb_is_gso(skb)) {
+ if (IP6CB(skb)->frag_max_size) {
+ /* frag_max_size tell us that, this packet have been
+ * defragmented by netfilter IPv6 conntrack module.
+ */
+ if (IP6CB(skb)->frag_max_size > mtu)
+ return true; /* largest fragment violate MTU */
+ }
+ else if (skb->len > mtu && !skb_is_gso(skb)) {
return true; /* Packet size violate MTU size */
}
return false;
^ permalink raw reply related
* [PATCH V2 1/2] ipvs: IPv6 MTU checking cleanup and bugfix
From: Jesper Dangaard Brouer @ 2012-08-28 14:22 UTC (permalink / raw)
To: netdev, Patrick McHardy, lvs-devel, Julian Anastasov,
Simon Horman, Pablo Neira Ayuso
Cc: Jesper Dangaard Brouer, Hans Schillstrom, Wensong Zhang,
netfilter-devel
In-Reply-To: <20120828142051.11475.63775.stgit@dragon>
Cleaning up the IPv6 MTU checking in the IPVS xmit code, by using
a common helper function __mtu_check_toobig_v6().
The MTU check for tunnel mode can also use this helper as
ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) is qual to
skb->len. And the 'mtu' variable have been adjusted before
calling helper.
Notice, this also fixes a bug, as the the MTU check in ip_vs_dr_xmit_v6()
were missing a check for skb_is_gso().
This bug e.g. caused issues for KVM IPVS setups, where different
Segmentation Offloading techniques are utilized, between guests,
via the virtio driver. This resulted in very bad performance,
due to the ICMPv6 "too big" messages didn't affect the sender.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
In V2: we also make tunnel mode use the common helper
net/netfilter/ipvs/ip_vs_xmit.c | 21 +++++++++++++++------
1 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 543a554..67a3978 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -85,6 +85,15 @@ __ip_vs_dst_check(struct ip_vs_dest *dest, u32 rtos)
return dst;
}
+static inline bool
+__mtu_check_toobig_v6(const struct sk_buff *skb, u32 mtu)
+{
+ if (skb->len > mtu && !skb_is_gso(skb)) {
+ return true; /* Packet size violate MTU size */
+ }
+ return false;
+}
+
/* Get route to daddr, update *saddr, optionally bind route to saddr */
static struct rtable *do_output_route4(struct net *net, __be32 daddr,
u32 rtos, int rt_mode, __be32 *saddr)
@@ -491,7 +500,7 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu && !skb_is_gso(skb)) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -712,7 +721,7 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu && !skb_is_gso(skb)) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -946,8 +955,8 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
if (skb_dst(skb))
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
- if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
- !skb_is_gso(skb)) {
+ /* MTU checking: Notice that 'mtu' have been adjusted before hand */
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -1113,7 +1122,7 @@ ip_vs_dr_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -1349,7 +1358,7 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu && !skb_is_gso(skb)) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
^ permalink raw reply related
* [PATCH V2 0/2] net: ipvs and netfilter IPv6 defrag MTU handling
From: Jesper Dangaard Brouer @ 2012-08-28 14:21 UTC (permalink / raw)
To: netdev, Patrick McHardy, lvs-devel, Julian Anastasov,
Simon Horman, Pablo Neira Ayuso
Cc: Jesper Dangaard Brouer, Hans Schillstrom, Wensong Zhang,
netfilter-devel
In-Reply-To: <Pine.GSO.4.63.1208281020530.18770@stinky-local.trash.net>
The following patchset V2 makes IPVS compatible with Patrick McHardys
IPv6 NAT patchset: http://thread.gmane.org/gmane.linux.network/239615
Specifically the part that improves defragmentation.
Patch01: Cleanup (and fixes a bug) in IPVS MTU IPv6 handling
In V2: we also handle tunnel mode, to use the common helper
Patch02: Extend MTU check to account for IPv6 NAT defrag changes
In V2: the tunnel mode is no longer a special case.
This patchset is based upon:
Homes ipvs-next tree:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git
BUT with Patrick McHardy's IPv6 NAT patchset applied first, on top of
commit: 3654e61137db891f5312e6dd813b961484b5fdf3
title: "ipvs: add pmtu_disc option to disable IP DF for TUN packets"
---
Jesper Dangaard Brouer (2):
ipvs: Extend MTU check to account for IPv6 NAT defrag changes
ipvs: IPv6 MTU checking cleanup and bugfix
net/netfilter/ipvs/ip_vs_xmit.c | 28 ++++++++++++++++++++++------
1 files changed, 22 insertions(+), 6 deletions(-)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] cxgb3: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 14:19 UTC (permalink / raw)
To: David.Laight; +Cc: divy, yongjun_wei, netdev
On 08/28/2012 09:24 PM, David Laight wrote:
>> - if (t->nofail_skb)
>> - kfree_skb(t->nofail_skb);
>> + kfree_skb(t->nofail_skb);
> Some of these conditionals might be there for performance
> reasons - eg if common path and normally NULL.
I have a quick look at those code, cxgb3_offload_deactivate()
is only used when remove device or device error and then disable
the device, so maybe it is not a common path.
^ permalink raw reply
* Re: [PATCH] skbuff: remove pointless conditional before kfree_skb()
From: Eric Dumazet @ 2012-08-28 14:12 UTC (permalink / raw)
To: Wei Yongjun; +Cc: davem, yongjun_wei, netdev
In-Reply-To: <CAPgLHd9m2VbFFXt5Q3MMrK6BPXvMrT-ptq+RfgBToXOpxAKOOw@mail.gmail.com>
On Tue, 2012-08-28 at 21:10 +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Remove pointless conditional before kfree_skb().
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> include/linux/skbuff.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 7632c87..0b846d9 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2464,8 +2464,7 @@ static inline void nf_conntrack_get_reasm(struct sk_buff *skb)
> }
> static inline void nf_conntrack_put_reasm(struct sk_buff *skb)
> {
> - if (skb)
> - kfree_skb(skb);
> + kfree_skb(skb);
> }
> #endif
> #ifdef CONFIG_BRIDGE_NETFILTER
>
Its not exactly pointless.
Its a tradeoff between kernel code size, and ability for cpu to predict
a branch in kfree_skb()
This test is in hot path, and therefore this patch can potentially have
a performance impact.
I really think most kfree_skb() calls are done with a non NULL param,
so the branch prediction is good.
But after this patch, things are totally different.
Therefore, I am against it.
^ permalink raw reply
* RE: [PATCH] cxgb3: remove pointless conditional before kfree_skb()
From: David Laight @ 2012-08-28 13:24 UTC (permalink / raw)
To: Wei Yongjun, divy; +Cc: yongjun_wei, netdev
In-Reply-To: <CAPgLHd9b+0DDZ9iYFzNS4NP1w_3GmWaMD+ihqeVeEGXei9cfUg@mail.gmail.com>
> - if (t->nofail_skb)
> - kfree_skb(t->nofail_skb);
> + kfree_skb(t->nofail_skb);
Some of these conditionals might be there for performance
reasons - eg if common path and normally NULL.
David
^ permalink raw reply
* [PATCH] mISDN: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:12 UTC (permalink / raw)
To: isdn; +Cc: yongjun_wei, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/isdn/mISDN/socket.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index abe2d69..f677724 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -243,8 +243,7 @@ mISDN_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
}
done:
- if (skb)
- kfree_skb(skb);
+ kfree_skb(skb);
release_sock(sk);
return err;
}
^ permalink raw reply related
* [PATCH] rtlwifi: rtl8192c: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:12 UTC (permalink / raw)
To: Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ,
chaoming_li-kXabqFNEczNtrwSWzY7KCg,
linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
---
drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c b/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
index 44febfd..b627151 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
@@ -577,8 +577,7 @@ static bool _rtl92c_cmd_send_packet(struct ieee80211_hw *hw,
ring = &rtlpci->tx_ring[BEACON_QUEUE];
pskb = __skb_dequeue(&ring->queue);
- if (pskb)
- kfree_skb(pskb);
+ kfree_skb(pskb);
spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] rtlwifi: rtl8192de: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:11 UTC (permalink / raw)
To: Larry.Finger, chaoming_li, linville; +Cc: yongjun_wei, linux-wireless, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/wireless/rtlwifi/rtl8192de/fw.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/fw.c b/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
index 895ae6c..a3aede0 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/fw.c
@@ -570,8 +570,7 @@ static bool _rtl92d_cmd_send_packet(struct ieee80211_hw *hw,
ring = &rtlpci->tx_ring[BEACON_QUEUE];
pskb = __skb_dequeue(&ring->queue);
- if (pskb)
- kfree_skb(pskb);
+ kfree_skb(pskb);
spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
pdesc = &ring->desc[idx];
/* discard output from call below */
^ permalink raw reply related
* [PATCH] skbuff: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:10 UTC (permalink / raw)
To: davem; +Cc: yongjun_wei, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
include/linux/skbuff.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7632c87..0b846d9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2464,8 +2464,7 @@ static inline void nf_conntrack_get_reasm(struct sk_buff *skb)
}
static inline void nf_conntrack_put_reasm(struct sk_buff *skb)
{
- if (skb)
- kfree_skb(skb);
+ kfree_skb(skb);
}
#endif
#ifdef CONFIG_BRIDGE_NETFILTER
^ permalink raw reply related
* [PATCH] batman-adv: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:06 UTC (permalink / raw)
To: lindner_marek, siwu, ordex, davem; +Cc: yongjun_wei, b.a.t.m.a.n, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
net/batman-adv/send.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index 3b4b2da..534dc46 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -99,8 +99,7 @@ void batadv_schedule_bat_ogm(struct batadv_hard_iface *hard_iface)
static void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet)
{
- if (forw_packet->skb)
- kfree_skb(forw_packet->skb);
+ kfree_skb(forw_packet->skb);
if (forw_packet->if_incoming)
batadv_hardif_free_ref(forw_packet->if_incoming);
kfree(forw_packet);
^ permalink raw reply related
* [PATCH] tipc: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:05 UTC (permalink / raw)
To: jon.maloy, allan.stephens, davem; +Cc: yongjun_wei, netdev, tipc-discussion
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
net/tipc/port.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/tipc/port.c b/net/tipc/port.c
index 07c42fb..9227d2c 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -770,8 +770,7 @@ static void port_dispatcher_sigh(void *dummy)
break;
}
}
- if (buf)
- kfree_skb(buf);
+ kfree_skb(buf);
buf = next;
continue;
err:
@@ -819,8 +818,7 @@ err:
break;
}
}
- if (buf)
- kfree_skb(buf);
+ kfree_skb(buf);
buf = next;
continue;
reject:
^ permalink raw reply related
* [PATCH] cxgb3: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:04 UTC (permalink / raw)
To: divy; +Cc: yongjun_wei, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 3 +--
drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
index 2dbbcbb..aac26f2 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
@@ -1341,8 +1341,7 @@ void cxgb3_offload_deactivate(struct adapter *adapter)
rcu_read_unlock();
RCU_INIT_POINTER(tdev->l2opt, NULL);
call_rcu(&d->rcu_head, clean_l2_data);
- if (t->nofail_skb)
- kfree_skb(t->nofail_skb);
+ kfree_skb(t->nofail_skb);
kfree(t);
}
diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index 6505070..0b79309 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -3412,8 +3412,7 @@ static void __devexit remove_one(struct pci_dev *pdev)
free_netdev(adapter->port[i]);
iounmap(adapter->regs);
- if (adapter->nofail_skb)
- kfree_skb(adapter->nofail_skb);
+ kfree_skb(adapter->nofail_skb);
kfree(adapter);
pci_release_regions(pdev);
pci_disable_device(pdev);
^ permalink raw reply related
* [PATCH] NFC: remove pointless conditional before kfree_skb()
From: Wei Yongjun @ 2012-08-28 13:02 UTC (permalink / raw)
To: lauro.venancio, aloisio.almeida, sameo, davem
Cc: yongjun_wei, linux-wireless, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove pointless conditional before kfree_skb().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
net/nfc/hci/shdlc.c | 3 +--
net/nfc/hci/core.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/nfc/hci/shdlc.c b/net/nfc/hci/shdlc.c
index 6f840c1..52e5cbb 100644
--- a/net/nfc/hci/shdlc.c
+++ b/net/nfc/hci/shdlc.c
@@ -241,8 +241,7 @@ static void nfc_shdlc_rcv_i_frame(struct nfc_shdlc *shdlc,
}
exit:
- if (skb)
- kfree_skb(skb);
+ kfree_skb(skb);
}
static void nfc_shdlc_rcv_ack(struct nfc_shdlc *shdlc, int y_nr)
diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index 1ac7b3f..35413cc 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -398,8 +398,7 @@ disconnect_all:
nfc_hci_disconnect_all_gates(hdev);
exit:
- if (skb)
- kfree_skb(skb);
+ kfree_skb(skb);
return r;
}
^ permalink raw reply related
* [PATCH] net: qmi_wwan: new device: Foxconn/Novatel E396
From: Aleksander Morgado @ 2012-08-28 12:30 UTC (permalink / raw)
To: netdev
Cc: linux-usb, Aleksander Morgado, Dan Williams, Bjørn Mork,
Ben Chan
Foxconn-branded Novatel E396, Gobi3k modem.
Cc: Dan Williams <dcbw@redhat.com>
Cc: Bjørn Mork <bjorn@mork.no>
Cc: Ben Chan <benchan@google.com>
Signed-off-by: Aleksander Morgado <aleksander@lanedo.com>
---
drivers/net/usb/qmi_wwan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 328397c..189e52d 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -441,6 +441,7 @@ static const struct usb_device_id products[] = {
{QMI_GOBI_DEVICE(0x1199, 0x9015)}, /* Sierra Wireless Gobi 3000 Modem device */
{QMI_GOBI_DEVICE(0x1199, 0x9019)}, /* Sierra Wireless Gobi 3000 Modem device */
{QMI_GOBI_DEVICE(0x1199, 0x901b)}, /* Sierra Wireless MC7770 */
+ {QMI_GOBI_DEVICE(0x1410, 0xa021)}, /* Foxconn Gobi 3000 Modem device (Novatel E396) */
{ } /* END */
};
--
1.7.11.4
^ permalink raw reply related
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-08-28 11:56 UTC (permalink / raw)
To: Sasha Levin
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, Tejun Heo,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <503CAB1E.5010408-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> On 08/28/2012 12:11 PM, Mathieu Desnoyers wrote:
> > * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> >> On 08/25/2012 06:24 AM, Mathieu Desnoyers wrote:
> >>> * Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org) wrote:
> >>>> Hello,
> >>>>
> >>>> On Sat, Aug 25, 2012 at 12:59:25AM +0200, Sasha Levin wrote:
> >>>>> Thats the thing, the amount of things of things you can do with a given bucket
> >>>>> is very limited. You can't add entries to any point besides the head (without
> >>>>> walking the entire list).
> >>>>
> >>>> Kinda my point. We already have all the hlist*() interface to deal
> >>>> with such cases. Having something which is evidently the trivial
> >>>> hlist hashtable and advertises as such in the interface can be
> >>>> helpful. I think we need that more than we need anything fancy.
> >>>>
> >>>> Heh, this is a debate about which one is less insignificant. I can
> >>>> see your point. I'd really like to hear what others think on this.
> >>>>
> >>>> Guys, do we want something which is evidently trivial hlist hashtable
> >>>> which can use hlist_*() API directly or do we want something better
> >>>> encapsulated?
> >>>
> >>> My 2 cents, FWIW: I think this specific effort should target a trivially
> >>> understandable API and implementation, for use-cases where one would be
> >>> tempted to reimplement his own trivial hash table anyway. So here
> >>> exposing hlist internals, with which kernel developers are already
> >>> familiar, seems like a good approach in my opinion, because hiding stuff
> >>> behind new abstraction might make the target users go away.
> >>>
> >>> Then, as we see the need, we can eventually merge a more elaborate hash
> >>> table with poneys and whatnot, but I would expect that the trivial hash
> >>> table implementation would still be useful. There are of course very
> >>> compelling reasons to use a more featureful hash table: automatic
> >>> resize, RT-aware updates, scalable updates, etc... but I see a purpose
> >>> for a trivial implementation. Its primary strong points being:
> >>>
> >>> - it's trivially understandable, so anyone how want to be really sure
> >>> they won't end up debugging the hash table instead of their
> >>> work-in-progress code can have a full understanding of it,
> >>> - it has few dependencies, which makes it easier to understand and
> >>> easier to use in some contexts (e.g. early boot).
> >>>
> >>> So I'm in favor of not overdoing the abstraction for this trivial hash
> >>> table, and honestly I would rather prefer that this trivial hash table
> >>> stays trivial. A more elaborate hash table should probably come as a
> >>> separate API.
> >>>
> >>> Thanks,
> >>>
> >>> Mathieu
> >>>
> >>
> >> Alright, let's keep it simple then.
> >>
> >> I do want to keep the hash_for_each[rcu,safe] family though.
> >
> > Just a thought: if the API offered by the simple hash table focus on
> > providing a mechanism to find the hash bucket to which belongs the hash
> > chain containing the key looked up, and then expects the user to use the
> > hlist API to iterate on the chain (with or without the hlist _rcu
> > variant), then it might seem consistent that a helper providing
> > iteration over the entire table would actually just provide iteration on
> > all buckets, and let the user call the hlist for each iterator for each
> > node within the bucket, e.g.:
> >
> > struct hlist_head *head;
> > struct hlist_node *pos;
> >
> > hash_for_each_bucket(ht, head) {
> > hlist_for_each(pos, head) {
> > ...
> > }
> > }
> >
> > That way you only have to provide one single macro
> > (hash_for_each_bucket), and rely on the already existing:
> >
> > - hlist_for_each_entry
> > - hlist_for_each_safe
> > - hlist_for_each_entry_rcu
> > - hlist_for_each_safe_rcu
> > .....
> >
> > and various flavors that can appear in the future without duplicating
> > this API. So you won't even have to create _rcu, _safe, nor _safe_rcu
> > versions of the hash_for_each_bucket macro.
> >
> > Thoughts ?
>
> In my opinion, the downside here is that it'll require 2 function calls and 2
> levels of nesting for a simple hash iteration.
Those are macros, not functions. No function call is required. But I see
your point about nesting.
>
> hash_for_each_bucket() will always be followed by an iteration of that
> bucket, so splitting a hash_for_each() which does both into 2
> different functions which will almost always must be called in that
> given order sounds unintuitive to me.
>
> It's also just 3 different possible iterators:
>
> - hlist_for_each_entry
> - hlist_for_each_entry_safe
> - hlist_for_each_entry_rcu
>
> So I think that it's a good price to pay - 2 extra macro definitions
> in the header to save a macro call + nesting level in each place that
> uses a hashtable.
I must admin I don't care that much one way or another.
Thanks,
Mathieu
>
>
> Thanks,
> Sasha
>
> > Thanks,
> >
> > Mathieu
> >
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-28 11:27 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Tejun Heo, torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-X9Un+BFzKDI, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, josh-iaAMLnmF4UmaiuxdJuQwMA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <20120828101148.GA21683@Krystal>
On 08/28/2012 12:11 PM, Mathieu Desnoyers wrote:
> * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
>> On 08/25/2012 06:24 AM, Mathieu Desnoyers wrote:
>>> * Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org) wrote:
>>>> Hello,
>>>>
>>>> On Sat, Aug 25, 2012 at 12:59:25AM +0200, Sasha Levin wrote:
>>>>> Thats the thing, the amount of things of things you can do with a given bucket
>>>>> is very limited. You can't add entries to any point besides the head (without
>>>>> walking the entire list).
>>>>
>>>> Kinda my point. We already have all the hlist*() interface to deal
>>>> with such cases. Having something which is evidently the trivial
>>>> hlist hashtable and advertises as such in the interface can be
>>>> helpful. I think we need that more than we need anything fancy.
>>>>
>>>> Heh, this is a debate about which one is less insignificant. I can
>>>> see your point. I'd really like to hear what others think on this.
>>>>
>>>> Guys, do we want something which is evidently trivial hlist hashtable
>>>> which can use hlist_*() API directly or do we want something better
>>>> encapsulated?
>>>
>>> My 2 cents, FWIW: I think this specific effort should target a trivially
>>> understandable API and implementation, for use-cases where one would be
>>> tempted to reimplement his own trivial hash table anyway. So here
>>> exposing hlist internals, with which kernel developers are already
>>> familiar, seems like a good approach in my opinion, because hiding stuff
>>> behind new abstraction might make the target users go away.
>>>
>>> Then, as we see the need, we can eventually merge a more elaborate hash
>>> table with poneys and whatnot, but I would expect that the trivial hash
>>> table implementation would still be useful. There are of course very
>>> compelling reasons to use a more featureful hash table: automatic
>>> resize, RT-aware updates, scalable updates, etc... but I see a purpose
>>> for a trivial implementation. Its primary strong points being:
>>>
>>> - it's trivially understandable, so anyone how want to be really sure
>>> they won't end up debugging the hash table instead of their
>>> work-in-progress code can have a full understanding of it,
>>> - it has few dependencies, which makes it easier to understand and
>>> easier to use in some contexts (e.g. early boot).
>>>
>>> So I'm in favor of not overdoing the abstraction for this trivial hash
>>> table, and honestly I would rather prefer that this trivial hash table
>>> stays trivial. A more elaborate hash table should probably come as a
>>> separate API.
>>>
>>> Thanks,
>>>
>>> Mathieu
>>>
>>
>> Alright, let's keep it simple then.
>>
>> I do want to keep the hash_for_each[rcu,safe] family though.
>
> Just a thought: if the API offered by the simple hash table focus on
> providing a mechanism to find the hash bucket to which belongs the hash
> chain containing the key looked up, and then expects the user to use the
> hlist API to iterate on the chain (with or without the hlist _rcu
> variant), then it might seem consistent that a helper providing
> iteration over the entire table would actually just provide iteration on
> all buckets, and let the user call the hlist for each iterator for each
> node within the bucket, e.g.:
>
> struct hlist_head *head;
> struct hlist_node *pos;
>
> hash_for_each_bucket(ht, head) {
> hlist_for_each(pos, head) {
> ...
> }
> }
>
> That way you only have to provide one single macro
> (hash_for_each_bucket), and rely on the already existing:
>
> - hlist_for_each_entry
> - hlist_for_each_safe
> - hlist_for_each_entry_rcu
> - hlist_for_each_safe_rcu
> .....
>
> and various flavors that can appear in the future without duplicating
> this API. So you won't even have to create _rcu, _safe, nor _safe_rcu
> versions of the hash_for_each_bucket macro.
>
> Thoughts ?
In my opinion, the downside here is that it'll require 2 function calls and 2
levels of nesting for a simple hash iteration.
hash_for_each_bucket() will always be followed by an iteration of that bucket,
so splitting a hash_for_each() which does both into 2 different functions which
will almost always must be called in that given order sounds unintuitive to me.
It's also just 3 different possible iterators:
- hlist_for_each_entry
- hlist_for_each_entry_safe
- hlist_for_each_entry_rcu
So I think that it's a good price to pay - 2 extra macro definitions in the
header to save a macro call + nesting level in each place that uses a hashtable.
Thanks,
Sasha
> Thanks,
>
> Mathieu
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox