Hello David,
>
> during the drive to work it occurred to me that we should basically have
> the same logic around whether we should insert or replace or do nothing
> in both the smr and mutex critical sections.
>
> it at least makes the code easier to understand. i think?
yes, the new
Hello,
re-sending the same diff updated to current if_pfsync.c.
diff avoids a recursion to PF_LOCK(), which is indicated
by panic stack here:
> > production firewalls panicked with the pf lock trying to lock against
> > itself a couple of nights ago:
> >
> > db_enter() at db_enter+0x5
> >
Hello,
skip reading if you are not interested in L2 switching combined
with bluhm's diff [1], which enables parallel forwarding.
Hrvoje gave it a try and soon discovered some panics. Diff below
fixes a panic indicated by stack as follows:
login: panic: kernel diagnostic assertion
Hello,
just for the record...
> > in current tree the ether_input() is protected by NET_LOCK(), which is
> > grabbed
> > by caller as a writer. bluhm's diff changes NET_LOCK() readlock, so
> > ether_input() can run concurrently. Switching NET_LOCK() to r-lock has
> > implications on smr read
Hrvoje,
managed to trigger diagnostic panics with diff [1] sent by bluhm@
some time ago. The panic Hrvoje sees comes from ether_input() here:
414
415 /*
416 * Third phase: bridge processing.
417 *
418 * Give the packet to a bridge interface, ie,
Hello,
On Mon, May 17, 2021 at 06:59:36PM +0200, Martin Pieuchot wrote:
> On 17/05/21(Mon) 16:24, Alexandr Nedvedicky wrote:
> > Hrvoje,
> >
> > managed to trigger diagnostic panics with diff [1] sent by bluhm@
> > some time ago. The panic Hrvoje sees comes from ether_
On Mon, May 17, 2021 at 08:27:12PM +0200, Martin Pieuchot wrote:
> On 17/05/21(Mon) 19:52, Alexandr Nedvedicky wrote:
> > [...]
> > I don't mind to trade pf_lock and pf_state_lock for mutexes, however
> > I see such step is kind of 'NO-OP'. We do have sufficient measure
Hello,
On Mon, May 17, 2021 at 08:19:37PM +0200, Alexander Bluhm wrote:
> On Mon, May 17, 2021 at 04:24:15PM +0200, Alexandr Nedvedicky wrote:
> > in current tree the ether_input() is protected by NET_LOCK(), which is
> > grabbed
> > by caller as a writer. bluhm's
Hello,
>
>
> with this diff i can't reproduce panic as before. i tried pf with
> routing, veb, tpmr and bridge.
>
> Do you want me to test this diff with parallel diff?
>
I think it makes no sense to test the change around copyout() with parallel
diff as mpi@ points out rwlocks don't
Hello,
On Thu, May 20, 2021 at 11:28:19AM +0200, Claudio Jeker wrote:
>
> One way to reduce the problems with copyout(9) is to use uvm_vslock() to
> lock the pages. This is what sysctl does but it comes with its own set of
> issues.
>
using uvm_vslock() did not come to my mind, when I was
Hello,
On Mon, May 31, 2021 at 02:33:00PM +1000, David Gwynne wrote:
> if you're looking at an ip header, it makes sense to do some checks to
> make sure that the values and addresses make some sense. the canonical
> versions of these checks are in the ipv4 and ipv6 input paths, which
> makes
Hello David,
>
> my best argument for mutexes in pf is that currently we use smr critical
> sections around things like the bridge and aggr port selection, which
> are very obviously read-only workloads. pf may be a largely read-only
> workload, but where it is at the moment means it's not
Hello David,
> the scope of the pf locks likely needs reduction anyway. one of my
I agree. pf_lock covers too much in PF currently. it protects,
all rules, tables and fragment caches.
> production firewalls panicked with the pf lock trying to lock against
> itself a couple of nights
Hello,
On Sun, Jun 06, 2021 at 09:54:50PM +0200, Hrvoje Popovski wrote:
>
>
> this one?
yes, exactly this one.
>
>
> ddb{5}> show panic
> cpu5: kernel diagnostic assertion "rvebe == NULL" failed: file
> "/sys/net/if_etherbridge.c", line 226 ebt_replace eb 0x80682e68
> nebe
On Wed, Jul 07, 2021 at 01:15:00PM -0700, Chris Cappuccio wrote:
> Alexandr Nedvedicky [alexandr.nedvedi...@oracle.com] wrote:
> > diff --git a/sys/net/if_tpmr.c b/sys/net/if_tpmr.c
> > index f6eb99f347c..4ffa5b18293 100644
> > @@ -725,10 +759,9 @@ tpmr_p_dtor(struct t
Hello,
On Wed, Jul 07, 2021 at 06:14:35PM +0200, Alexander Bluhm wrote:
> On Wed, Jul 07, 2021 at 10:20:01AM +0200, Alexandr Nedvedicky wrote:
> > we still need to agree on whether pf_test() can sleep (give up CPU),
> > when processing packet. I don't mind if pf_test
On Tue, Jul 06, 2021 at 11:05:47PM +0200, Alexander Bluhm wrote:
> Hi,
>
> Thank a lot to Hrvoje Popovski for testing my diff and to sashan@
> and dlg@ for fixing all the fallout in pf and pseudo drivers.
>
> Are there any bugs left? I think everything has been fixed.
>
I've just
Hello David,
>
> i think we can get away with not refcounting eb_entry structures at all.
> either they're in the etherbridge map/table or they're not, and the
> thing that takes them out of the map while holding the eb_lock mutex
> becomes responsible for their cleanup.
>
> i feel like most
Hello,
>
> But the functions in_rtchange() and in_pcbrtentry() do not touch
> t_maxseg. So orig_maxseg is always equal to tp->t_maxseg and
> tcp_output() is not called after changes in t_maxseg.
this also matches my observation.
>
> ok?
>
tcp_mtudisc() looks good to me now.
OK
Hello,
On Tue, Jul 06, 2021 at 11:05:47PM +0200, Alexander Bluhm wrote:
> Hi,
>
> Thank a lot to Hrvoje Popovski for testing my diff and to sashan@
> and dlg@ for fixing all the fallout in pf and pseudo drivers.
>
> Are there any bugs left? I think everything has been fixed.
>
there is
Hello,
Hrvoje gave a try to experimental diff, which trades rw-locks in pf(4)
for mutexes [1]. Hrvoje soon discovered machine panics, when doing 'pfctl -ss'
The callstack looks as follows:
panic: acquiring blockable sleep lock with spinlock or critical section
held (rwlock) vmmaplk
Stopped at
Hello,
> >
> > would it make sense to have let's say
> >
> > mq_move2mlist(struct mbuf_queue *, struct mbuf_list *)
>
> This already exists, it's called mq_delist()
>
thanks, I've missed that when skimming through sources.
> >
> > This would allow as to move whole
Hello,
people who will be running pf(4) with bluhm's diff [1], may trip
one of the asserts triggered by pf_state_key_link_reverse() here:
7366 void
7367 pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key
*skrev)
7368 {
7369 /* Note that sk and skrev may be equal,
Hello,
just a quick question:
was pf(4) enabled while running those tests?
if pf(4) was enabled while those tests were running,
what rules were loaded to to pf(4)?
my guess is pf(4) was not enabled when running those tests. if I remember
correctly I could see performance boost by
Hello,
> > Hi,
> >
> > with this diff i'm getting panic when i'm pushing traffic over that box.
> > This is plain forwarding. To compile with witness ?
>
>
> with witness
>
any chance to check other CPUs to see what code they are executing?
I hope to be lucky enough and see thread,
Hello,
thanks for good news.
On Wed, Apr 21, 2021 at 10:32:08PM +0200, Alexander Bluhm wrote:
> On Wed, Apr 21, 2021 at 09:59:53PM +0200, Alexandr Nedvedicky wrote:
> > was pf(4) enabled while running those tests?
>
> Yes.
>
> > if pf(4) was enabled while
Hello,
On Thu, Apr 22, 2021 at 01:09:34PM +0200, Alexander Bluhm wrote:
> On Thu, Apr 22, 2021 at 12:33:13PM +0200, Hrvoje Popovski wrote:
> > r620-1# papnpaiancini:cc :p :op
> > opooolo_llc_ac_caccahhceh_ei_eti_tieetmme_mm__amgamigacigci__cc_hccehhcekcekc::
> > k :m bmubmfubfuppflp llc pc pcuup
Hello,
On Thu, Apr 22, 2021 at 03:08:21PM +0200, Mark Kettenis wrote:
> > Date: Thu, 22 Apr 2021 14:43:24 +0200
> > From: Alexandr Nedvedicky
> >
> > Hello,
> >
> > On Thu, Apr 22, 2021 at 01:09:34PM +0200, Alexander Bluhm wrote:
> > > On Thu, Apr
Hello,
> Such a diff is below. I will test extensively towmorrow. If anyone
> wants to try now, be my guest.
>
> Is the comment correct?
I think comment is not quite right.
the packet gets inserted into la->la_mq via arpresolve(), which
is not protected by KERNEL_LOCK().
arpresolve()
Hello,
diff looks good to me, I've just found one cosmetic nit.
> @@ -683,21 +685,17 @@ arpcache(struct ifnet *ifp, struct ether
>
> la->la_asked = 0;
> la->la_refreshed = 0;
> - while ((m = mq_dequeue(>la_mq)) != NULL) {
> - unsigned int len;
> -
> -
Hello,
On Wed, Apr 28, 2021 at 01:49:27AM +0200, Alexander Bluhm wrote:
> On Wed, Apr 28, 2021 at 12:46:31AM +0200, Alexandr Nedvedicky wrote:
> > looks good in general. I have just two nits/questions.
>
> mvs@ has asked the same questions.
>
> > > struct llin
Hello,
On Tue, Apr 27, 2021 at 11:45:19PM +0200, Alexander Bluhm wrote:
> On Sun, Apr 25, 2021 at 06:08:17PM +1000, Jonathan Matthew wrote:
> > On Sun, Apr 25, 2021 at 09:44:16AM +0200, Alexandr Nedvedicky wrote:
> > This already exists, it's called mq_delist()
>
Hello,
looks good in general. I have just two nits/questions.
> ok?
>
> bluhm
>
> Index: netinet/if_ether.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.246
> diff -u -p -r1.246
Hello,
with moving towards NET_RLOCK...() shall we add an explicit
assert to state caller owns netlock exclusively? I propose
to introduce NET_ASSERT_WLOCKED()
NET_ASSERT_WLOCKED()
thanks and
regards
sashan
8<---8<---8<--8<
diff
Hello,
On Wed, Apr 28, 2021 at 02:25:26AM +0200, Alexander Bluhm wrote:
> On Wed, Apr 28, 2021 at 03:19:47AM +0300, Vitaliy Makkoveev wrote:
> > The code not only breaks loop but also cleans the queue. Should this be
> > kept?
>
this is a good point
> In IPv6 nd6_cache_lladdr() we have
On Wed, Apr 28, 2021 at 03:31:41PM +0200, Claudio Jeker wrote:
> On Wed, Apr 28, 2021 at 02:56:27PM +0200, Alexandr Nedvedicky wrote:
> > Hello,
> >
> > On Wed, Apr 28, 2021 at 02:25:26AM +0200, Alexander Bluhm wrote:
> > > On Wed, Apr 28, 2021 at 03:19:47AM
>
> This time arpcache() is only called by netisr() with both kernel and
> exclusive net locks held. RTM_DELETE message processing will also call
> ifp->if_rtrequest() with exclusive netlock held.
>
> So this while() loop within arpcache() can’t be broken by “arp -a -d”.
completely agree.
On Thu, Apr 29, 2021 at 02:30:48AM +0300, Vitaliy Makkoveev wrote:
>
>
> > On 29 Apr 2021, at 02:20, Alexandr Nedvedicky
> > wrote:
> >
> >
> >>
> >> This time arpcache() is only called by netisr() with both kernel and
> >> exclusiv
Hello,
I think this should go in as-is. Though I have one question/idea
to share at the moment.
> @@ -672,20 +666,18 @@ arpcache(struct ifnet *ifp, struct ether
>
> la->la_asked = 0;
> la->la_refreshed = 0;
> - while ((len = ml_len(>la_ml)) != 0) {
> - struct mbuf
hello,
On Fri, Jan 22, 2021 at 05:32:47PM +1000, David Gwynne wrote:
> I tried this diff, and it broke the ability to use dynamic addresses.
> ie, the following rules should work:
>
> pass in on gre52 inet proto icmp route-to (gre49:peer)
> pass in on vmx0 inet proto icmp route-to (gre:peer)
Hello Dominik,
thanks for reporting a bug. I'll take a look at it later today.
your proposed fix re-introduces a recursion to PF, which we want to avoid,
hence we let icmp_send() to dispatch outbound ICMP response to task.
We really need to fix ip_send() such the output task will handle IP
Hello,
> > 1) ip_insertoptions() does not update length of IP header (ip_hl)
> >
> > 2) ip_hl is being overridden anyway later in ip_output() called
> > by ip_send_dispatch() to send ICMP error packet out. Looks
> > like ip_send_dispatch() should use IP_RAWOUTPUT
Hello,
On Mon, Mar 29, 2021 at 11:56:40AM +, Schreilechner, Dominik wrote:
>
>
> > > I think the changes in ip_insertoptions() can be dropped completely,
> > > because the if-statement uses ip-ip_hl, which might not be initialized.
> >
> > yes, you are right, I think we should rather go
Hello,
> > @@ -790,6 +795,7 @@ ip_insertoptions(struct mbuf *m, struct mbuf *opt, int
> > *phlen)
> > memcpy(ip + 1, p->ipopt_list, optlen);
> > *phlen = sizeof(struct ip) + optlen;
> > ip->ip_len = htons(ntohs(ip->ip_len) + optlen);
> > + ip->ip_hl += (optlen >>
Hello,
On Mon, Mar 29, 2021 at 08:44:26AM +, Schreilechner, Dominik wrote:
> Hi,
>
>
>
> > yes, you are right. below is updated diff I would like to commit.
> > >
> > > Appart from that, adding a special task seems the way to go.
> >
> > I think so too. Alternative way would be to
Hello,
>
> You missed the initialization of ipsendraw_mq. Otherwise, ICMP with and
> without IP options work for me with the diff.
>
> I don't know how this is usually handled here and it is probably a bit
> nit-picky, but I introduced a new function to remove the duplicate code
> in
Hello,
> We really need to fix ip_send() such the output task will handle IP options
> properly.
took a look at bug reported by Dominik earlier today. Looks like
there are two issues:
1) ip_insertoptions() does not update length of IP header (ip_hl)
2) ip_hl is being
Hello,
> > +
> Can you remove one of the double empty lines?
sure, updated diff is below.
> > +
> > + old_reverse = atomic_cas_ptr(>reverse, NULL, sk);
> > + if (old_reverse != NULL)
> > + KASSERT(old_reverse == sk);
> > + else
> > + pf_state_key_ref(sk);
> > }
>
Hello,
On Tue, Aug 31, 2021 at 02:40:57PM +1000, David Gwynne wrote:
> handling the "no" option with a token, and "yes" via a string made my
> eye twitch.
>
> ok? or is the helpful yyerror a nice feature?
>
I actually think it's a nice feature. below is output
for current pfctl we have
Hello,
On Fri, Aug 06, 2021 at 12:26:10AM +0200, Alexander Bluhm wrote:
> Hi,
>
> I have seen some stalling TCP connections while doing unidirectional
> throughput tests. The sending machine is doing zero window probes,
> but is not sending more data although the other machine announced
> that
Hello,
On Mon, Aug 09, 2021 at 01:17:27PM +0200, Alexander Bluhm wrote:
> On Fri, Aug 06, 2021 at 05:22:18PM +0200, Alexandr Nedvedicky wrote:
> > > Although I did not obverve it, there seems to be the same problem
> > > for snd_wl1 and rcv_up. For rcv_up I
On Sun, Oct 24, 2021 at 06:00:59PM +0200, Alexander Bluhm wrote:
> On Sun, Oct 24, 2021 at 02:52:45AM +0200, Alexander Bluhm wrote:
> > There are more m_pullup()s in IPsec input. Pass down the pointer
> > to the mbuf. At the end it will reach ip_deliver() which expects
> > a pointer to an mbuf
Hello,
On Fri, Dec 03, 2021 at 03:42:09PM +0100, Claudio Jeker wrote:
>
> See comments below.
>
>
> > +void
> > +pfi_group_delmember(const char *group, struct ifnet *ifp)
> > +{
> > + struct pfi_kif *gkif, *ikif;
> > +
> > + if ((gkif = pfi_kif_get(group, NULL)) == NULL ||
> > +
Hello,
> > updated diff is attached.
>
> One comment below but this diff is OK claudio@
>
> > thanks and
> > regards
> > sashan
> >
> > 8<---8<---8<--8<
> > void
> > pfi_xcommit(void)
> > {
> > - struct pfi_kif *p;
> > + struct
Hello,
it has turned out things are bit more complicated when it comes to interface
groups. diff below makes following scenario work for me.
we start with etc/pf.conf as follows:
# cat /etc/pf.conf
set skip on lo
set skip on test1
set skip on test2
Hello,
On Fri, Nov 26, 2021 at 04:32:59PM +1100, Jonathan Gray wrote:
> Stop building the kernel with -Wno-uninitialized on clang archs.
> This hides real problems like the recently fixed uninitialised memory
> use in pf and igc.
yes, please. I'd like to have the warning enabled.
I'm
Hello,
On Fri, Nov 26, 2021 at 10:14:47PM +1100, Jonathan Gray wrote:
> On Fri, Nov 26, 2021 at 12:04:21PM +0100, Alexandr Nedvedicky wrote:
> > Hello,
> >
> > On Fri, Nov 26, 2021 at 04:32:59PM +1100, Jonathan Gray wrote:
> > > Stop building the kernel with -Wn
Hello,
thank you for taking a look at my diff.
> > }
> >
> > - if (kif->pfik_ifp != NULL || kif->pfik_group != NULL || kif == pfi_all)
> > + if (kif->pfik_ifp != NULL || kif->pfik_group != NULL ||kif == pfi_all)
>
> Missing space over^^^
Hello,
On Fri, Nov 26, 2021 at 01:01:40PM +0100, Claudio Jeker wrote:
>
> One more thing to consider, I think the following test in pfi_set_flags():
>
> > + if ((p->pfik_flags_new != p->pfik_flags) &&
> > + (p->pfik_flagrefs == 0))
> > +
Hello,
back in Gouveia claudio@ complained 'set skip on ...' requires reloading
pf.conf, when new interface appears. Diff below should fix that.
the idea is to introduce yet another reference type (PFI_KIF_REF_FLAG) pfi_kif
objects. PFI_KIF_REF_FLAG type keeps pfi_kif object in pf's interface
ed from pfi_kif_get()
when flags were actually same.
thanks and
regards
sashan
On Wed, Nov 17, 2021 at 03:07:04AM +0100, Alexandr Nedvedicky wrote:
> Hello,
>
> back in Gouveia claudio@ complained 'set skip on ...' requires reloading
> pf.conf, when new interface appears. Dif
On Tue, Oct 26, 2021 at 11:55:23PM +0300, Vitaliy Makkoveev wrote:
> Just for consistency with the rest of kern/uipc_socket2.c
looks OK to me.
sashan
>
> Index: sys/kern/uipc_socket2.c
> ===
> RCS file:
Hello,
my statement here is just for the record. we should have a follow up
discussion in a different thread, which is yet to be started (when time
will come).
>
> > - Make ARP MP safe. Currently we need the kernel lock there or
> > it crashes. This creates latency for all kind of packets.
Hello,
On Tue, Feb 15, 2022 at 03:29:19PM +1000, David Gwynne wrote:
> consistently means we do the check in pf_rule_copyin() so both
> DIOCADDRULE and DIOCCHANGERULE have the prio values checked. this in
> turn prevents invalid prio values getting set on a rule via
> DIOCCHANGERULE, which in
Hello,
On Fri, Mar 11, 2022 at 09:40:00PM +0100, Alexander Bluhm wrote:
> Regress tested with witness, rebased to current, anyone?
>
> On Wed, Mar 02, 2022 at 07:13:09PM +0100, Alexander Bluhm wrote:
> > Hi,
> >
> > pf_socket_lookup() calls in_pcbhashlookup() in the PCB layer. So
> > to run pf
Hello,
changes looks good. just few nits.
I took a closer look at ipsec_delete_policy():
667 ipsec_delete_policy(struct ipsec_policy *ipo)
668 {
669 struct ipsec_acquire *ipa;
670 struct radix_node_head *rnh;
671
On Wed, Mar 16, 2022 at 04:11:43PM +, Visa Hankala wrote:
> Use the refcnt API in bpf.
>
> OK?
Looks OK to me.
sashan
Hello,
> > > +ipsp_delete_acquire_locked(struct ipsec_acquire *ipa)
> > > +{
> > > + if (timeout_del(>ipa_timeout) == 1)
> > > + refcnt_rele(>ipa_refcnt);
> >^^
> > can we also put ASSERT/check into this branch
> > to verify we are no releasing the
Hello,
On Tue, Mar 15, 2022 at 12:58:48AM +0300, Vitaliy Makkoveev wrote:
> >
> >I think local var `ipa` needs to be initialized to NULL
> >to avoid random value/pointer when no `ipa` for given `seq`
> >is found.
> >
> >ipsp_pending_acquire() plays the same gamble.
>
> #define
Hello,
On Tue, Mar 15, 2022 at 12:37:00AM +0300, Vitaliy Makkoveev wrote:
> Hi,
>
> Why do you want to initialize `ipa’ variable in
> ipsp_pending_acquire() and ipsec_get_acquire()? This doesn’t
> require.
after looking at code with bluhm's diff applied I see this:
936 struct
Hello,
my original idea was to have a one mutex per pfsync queue. however it creates
things more complicated then necessary for queues, which keep pf(4) state.
hrvoje@ hit some panics in this area recently. bluhm@ and I are still looking
at those issues. However there is the first change in
Hello,
On Thu, Feb 24, 2022 at 10:42:58PM +0100, Alexander Bluhm wrote:
> Hi,
>
> Hrvoje reported some crashes with pfsync, IPsec and parallel
> forwarding. Some locks were missing around the tdb flags in pfsync.
>
> ok?
>
change looks good to me. I just have a single concern
about
Hello,
> > i will still play with sasyncd setup, maybe something comes up
> >
>
> And when I thought everything is fine.. after 2 days of hitting sasyncd
> setup I've got this panic .. will stay in ddb if some other information
> is needed
>
I think you are hitting a different issue,
Hello,
>
> do you need ddb console ? if you want i can try to reproduce this panic
> without isakmpd, just hitting pfsync setup?
>
>
console might help. I won't be able to get to it before tonight/tomorrow.
I think you can leave isakmpd enabled. it still might be a contributing
On Wed, Mar 02, 2022 at 12:47:11AM +0100, Alexander Bluhm wrote:
> Hi,
>
> The return value of in6_pcbnotify() is never used. Make it a void
> function.
>
> ok?
>
given we always return 0, which is nothing, then it makes sense to
return void, (which is also nothing).
ok sashan2
Hello,
On Mon, Mar 21, 2022 at 04:53:12PM +0100, Alexander Bluhm wrote:
> Hi,
>
> syzkaller and witness found a bug in my pcb table mutex commit.
>
> https://syzkaller.appspot.com/bug?id=90a4811c99d6a2df7b252971b754612ca632894d
>
> For multicast and broadcast packets udp_input() traverses the
Hello,
there was a brief email discussion off-list between bluhm, mbuhl and me
preceding diff below. Let me briefly summarize a context for tech@ here.
Moritz (mbuhl) is working on a fix reported syzkaller by syzkaller [1].
In order to fix it he wants to move both locks (pf_lock and net_lock)
On Wed, Mar 23, 2022 at 03:53:21PM +0100, Alexander Bluhm wrote:
> Hi,
>
> pfioctl() is inconsistent when to use break or goto fail. There
> is a big switch and when looking at a break you need more context
> to see where it jumps to.
>
> I would like to use goto fail consistently to leave the
Hello,
I'm fine with the fix. I have just small comment/nit. leaving it
up to you to fix it ore leave it.
> -
> if (newrule->rt && !newrule->direction) {
> pf_rule_free(newrule);
> error = EINVAL;
> @@ -3216,6
Hello,
On Thu, Apr 07, 2022 at 03:21:32PM +0200, Moritz Buhl wrote:
> Hi,
>
> Since 2015 pf_rollback_rules always returns 0. Make it void.
>
> OK?
looks good to me.
OK sashan
Hello,
Hrvoje was testing pf(4) and pfsync(4) with parallel forwarding diff
which bluhm@ has shared sometime ago.
Hrvoje found a bug in my very naive implementation of 'snapshots':
1573 void
1574 pfsync_grab_snapshot(struct pfsync_snapshot *sn, struct pfsync_softc *sc)
1575 {
1576 int
Hello,
thank you for taking a look at it.
>
> I think there is a use after free in you diff. After you return
> from pfsync_delete_tdb() you must not access the TDB again.
>
> Comments inline.
>
> >
> > TAILQ_INIT(>sn_upd_req_list);
> > - TAILQ_CONCAT(>sn_upd_req_list,
Hello,
I've applied diff to 7.3. it works I can paste to serial console,
hitting tab, etc.. Everything seems to work as expected.
thanks and
regards
sashan
On Mon, Oct 09, 2023 at 10:51:05AM -0400, Dave Voutila wrote:
> Looking for folks that use the serial console connection in vmd(8) and
>
Hello,
> When I check my pflog files in WireShark, I note that WireShark displays
> this in the "Info" column:
>
> [pass vio0/-1]
>
yes, this can be default rule. snippet below comes from pfattach():
239 /* default rule should never be garbage collected */
240
Hello,
On Mon, Oct 09, 2023 at 08:07:35PM +0200, Alexander Bluhm wrote:
> Hi,
>
> pf_pull_hdr() allows to pass an action pointer parameter as output
> value. This is never used, all callers pass a NULL argument. Remove
> ACTION_SET() entirely.
>
> The logic if (fragoff >= len) in
On Tue, Oct 10, 2023 at 02:53:15PM +0200, Alexander Bluhm wrote:
> Hi,
>
> The behaviour of the PFRULE_SRCTRACK and max_states check was
> unintentionally changed by this commit.
>
>
> revision 1.964
> date: 2016/01/25 18:49:57; author: sashan; state: Exp; lines:
Hello,
I'm fine with it.
OK sashan
On Wed, Oct 11, 2023 at 12:28:20AM +0200, Alexander Bluhm wrote:
> Hi,
>
> If a packet is malformed, it is dropped by pf(4). The rule referenced
> in pflog(4) is the default rule. As the default rule is a pass
> rule, tcpdump prints "pass" although the
Hello,
On Tue, Aug 22, 2023 at 12:21:59AM +0200, Alexander Bluhm wrote:
> On Thu, Aug 17, 2023 at 12:02:35PM +1000, David Gwynne wrote:
> > there are links between the pcb/socket layer and pf as an optimisation,
> > and links on mbufs between both sides of a forwarded connection.
> > these links
On Fri, Apr 08, 2022 at 12:56:12PM +0200, Alexander Bluhm wrote:
> Hi,
>
> I now the right time to commit the parallel forwarding diff?
>
> Known limitiations are:
> - Hrvoje has seen a crash with both pfsync and ipsec on his production
> machine. But he cannot reproduce it in his lab.
> -
Hello,
I must admit I'm still kind of lost here. The email from kettenis@
with comment from linux helped a bit, but still my understanding
is really blurry here. Let me share my view on the recent
example from visa@.
> >
> > Above, thread A would need a barrier before the release. Otherwise
>
Hello,
Looks like time has come to tweak a tuning knob from paranoia
towards 'get network working'
I have just few nits, see below.
thanks and
regards
sashan
>
> int
> +pf_walk_option(struct pf_pdesc *pd, struct ip *h, int off, int end,
> +u_short *reason)
> +{
> + uint8_t type,
Hello,
On Wed, Apr 20, 2022 at 06:14:37PM +0200, Alexander Bluhm wrote:
> Hi,
>
> In pfsync there are some KASSERT hidden behind #ifdef PFSYNC_DEBUG.
> That does not make sense to me. Either they are correct, then they
> should actively check in production. Or they got wrong over time,
> then
Hello,
On Wed, Apr 20, 2022 at 03:43:06PM +0200, Alexander Bluhm wrote:
> On Sat, Apr 09, 2022 at 01:51:05AM +0200, Alexandr Nedvedicky wrote:
> > updated diff is below.
>
> I am not sure what Hrvoje actually did test and what not. My
> impression was, that he got a panic
Hello,
On Thu, Apr 21, 2022 at 02:42:45PM +0200, Alexander Bluhm wrote:
> On Wed, Apr 20, 2022 at 11:22:27PM +0200, Alexandr Nedvedicky wrote:
> > updated diff is below
>
> OK bluhm@
>
> You have to merge again, as I removed #ifdef PFSYNC_DEBUG and added
> a #
Hello,
On Fri, May 27, 2022 at 10:33:06AM +0200, Hrvoje Popovski wrote:
> Hi all,
>
> I'm running firewall in production with NET_TASKQ=6 with claudio@ "use
> timeout for rttimer" and bluhm@ "kernel lock in arp" diffs.
> After week or so of running smoothly I've got panic.
thank you for
Hello,
On Tue, May 17, 2022 at 06:40:12PM +, Miod Vallat wrote:
> sys/net/pf.c r1.968 added a call to m_print() #ifdef DDB in a
> troublesome situation.
>
> Once the root cause of the problem was fixed, the DDB-specific code path
> was removed in r1.970, but the added includes were kept,
Hello,
I like the idea. I would just consider to make ip6_hbhchcheck()
to always free mp on error and set NULL as return value.
for example here in current ip6_input_if() we may leak memory
on error:
308 int
309 ip6_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet
*ifp)
Hello,
On Tue, Jun 28, 2022 at 02:39:24AM +0200, Alexander Bluhm wrote:
> Hi,
>
> The ip6_hbhchcheck() function never reads the nxtp parameter, it
> only sets its value. It is more obvious if we return the next
> protocol and return IPPROTO_DONE to signal error. All IP protocol
> functions do
Hello,
On Wed, Jun 29, 2022 at 02:34:02PM +0200, Alexander Bluhm wrote:
> On Wed, Jun 29, 2022 at 12:42:13AM +0200, Alexandr Nedvedicky wrote:
> > for example here in current ip6_input_if() we may leak memory
> > on error:
>
> Curently we do not leak memory. We free t
Hello,
I've spotted this glitch while hunting down use after-free
in 'veb' packet path. I believe the issue is rather hypothetical,
there is no evidence the deemed memory leak ever occurred.
Anyway I believe the if_vinput() should always consume packet
by either passing it further when
401 - 500 of 609 matches
Mail list logo
